Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FAB-17508] Removing viper library from viperutil #1511

Merged
merged 1 commit into from
Jul 1, 2020

Conversation

nbasker
Copy link
Contributor

@nbasker nbasker commented Jul 1, 2020

Type of change

Improvement (improvement to code by avoiding viper library that is not
needed in this context)

Description

Avoiding the overlap in the implementation of getKeysRecursively and
viper.Get() function. The sub-keys needed by getKeysRecursively is
readily available and the usage of viper.Get() can be avoided.

The reference to viper library is removed from the following modules

  1. orderer/common/localconfig
  2. internal/configtxgen/genesisconfig
  3. common/viperutil

A ConfigParser class is created in viperutil/config_utils.go to achieve
the needed funtionality.

Additional details

Unit tests done are the following

go test -race -count 1 -v -run Test github.com/hyperledger/fabric/orderer/common/localconfig | tail -2
PASS
ok github.com/hyperledger/fabric/orderer/common/localconfig
1.260s
go test -race -count 1 -v -run Test github.com/hyperledger/fabric/internal/configtxgen/genesisconfig | tail -2
PASS
ok github.com/hyperledger/fabric/internal/configtxgen/genesisconfig
1.115s
go test -race -count 1 -v -run Test github.com/hyperledger/fabric/common/viperutil | tail -2
PASS
ok github.com/hyperledger/fabric/common/viperutil 0.793s
go test -race -count 1 -v -run Test github.com/hyperledger/fabric/orderer/consensus/etcdraft | tail -2
PASS
ok github.com/hyperledger/fabric/orderer/consensus/etcdraft
52.528s
go test -race -count 1 -v -run Test github.com/hyperledger/fabric/orderer/consensus/etcdraft | grep SUCCESS!
SUCCESS! -- 107 Passed | 0 Failed | 0 Pending | 0 Skipped

Related issues

https://jira.hyperledger.org/browse/FAB-17508

Signed-off-by: NicholasB nbasker@gmail.com

Type of change

  • Improvement (improvement to code, performance, etc)

Description

Avoiding the overlap in the implementation of getKeysRecursively and
viper.Get() function.

Additional details

Unit Tests Done

Related issues

https://jira.hyperledger.org/browse/FAB-17508

Type of change

Improvement (improvement to code by avoiding viper library that is not
needed in this context)

Description

Avoiding the overlap in the implementation of getKeysRecursively and
viper.Get() function. The sub-keys needed by getKeysRecursively is
readily available and the usage of viper.Get() can be avoided.

The reference to viper library is removed from the following modules
1. orderer/common/localconfig
2. internal/configtxgen/genesisconfig
3. common/viperutil

A ConfigParser class is created in viperutil/config_utils.go to achieve
the needed funtionality.

Additional details

Unit tests done are the following

go test -race -count 1 -v -run Test github.com/hyperledger/fabric/orderer/common/localconfig | tail -2
PASS
ok      github.com/hyperledger/fabric/orderer/common/localconfig
1.260s
go test -race -count 1 -v -run Test github.com/hyperledger/fabric/internal/configtxgen/genesisconfig | tail -2
PASS
ok      github.com/hyperledger/fabric/internal/configtxgen/genesisconfig
1.115s
go test -race -count 1 -v -run Test github.com/hyperledger/fabric/common/viperutil | tail -2
PASS
ok      github.com/hyperledger/fabric/common/viperutil  0.793s
go test -race -count 1 -v -run Test github.com/hyperledger/fabric/orderer/consensus/etcdraft | tail -2
PASS
ok      github.com/hyperledger/fabric/orderer/consensus/etcdraft
52.528s
go test -race -count 1 -v -run Test github.com/hyperledger/fabric/orderer/consensus/etcdraft | grep SUCCESS!
SUCCESS! -- 107 Passed | 0 Failed | 0 Pending | 0 Skipped

Related issues

https://jira.hyperledger.org/browse/FAB-17508

Signed-off-by: NicholasB <nbasker@gmail.com>
@nbasker nbasker requested a review from a team as a code owner July 1, 2020 07:18
@nbasker
Copy link
Contributor Author

nbasker commented Jul 1, 2020

@sykesm apologies as I had to create a separate new PR. Prior to doing a squash in the previous PR, I synced fabric/master to bug-fix branch and inadvertently during squash failed to drop other commits. Further tried git rebase --onto option and messed up my bug-fix branch. I tried a few things but was unable to recover, so created a new branch.

Unfortunately the review history is not here in this PR, but available at #1392. I have double checked and made sure that every review comment is addressed here.

@sykesm sykesm merged commit d41dafc into hyperledger:master Jul 1, 2020
@nbasker nbasker deleted the viper_cleanup branch July 22, 2020 11:14
kopaygorodsky added a commit to kopaygorodsky/fabric that referenced this pull request Sep 27, 2021
…riable. Cherry-picked hyperledger#1511, upgraded to viper v1.1.1 and used UnmarshalKey to retrieve peer BCCSP config

Signed-off-by: Vladyslav Kopaihorodskyi <vlad.kopaygorodsky@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants