Skip to content

Commit

Permalink
Overwrite default config with CLI input even when the input value is …
Browse files Browse the repository at this point in the history
…considered as empty

This change will fix two issues:
1. When CLI sets a flag to an empty value in Go, e.g. 0 as to Uint64, the flag will be skipped and never set correctly. This problem could be solved by using option "WithOverwriteWithEmptyValue" when merge two configs.
2. The default non-empty value in server config will be overwritten to an empty value after flag initialization. This problem is solved by explicitly providing default value to all flags that have a default value option.
  • Loading branch information
cffls committed Jun 1, 2022
1 parent 4cec5f0 commit 96ac8d1
Show file tree
Hide file tree
Showing 3 changed files with 366 additions and 298 deletions.
7 changes: 1 addition & 6 deletions internal/cli/server/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -956,14 +956,9 @@ func (c *Config) buildNode() (*node.Config, error) {

func (c *Config) Merge(cc ...*Config) error {
for _, elem := range cc {
if err := mergo.Merge(c, elem, mergo.WithOverride, mergo.WithAppendSlice); err != nil {
if err := mergo.Merge(c, elem, mergo.WithOverwriteWithEmptyValue, mergo.WithAppendSlice); err != nil {
return fmt.Errorf("failed to merge configurations: %v", err)
}

// override max peers
if elem.P2P.MaxPeers == 0 {
c.P2P.MaxPeers = 0
}
}
return nil
}
Expand Down
5 changes: 1 addition & 4 deletions internal/cli/server/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,11 @@ func TestConfigMerge(t *testing.T) {
}
expected := &Config{
Chain: "1",
NoSnapshot: true,
NoSnapshot: false,
RequiredBlocks: map[string]string{
"a": "b",
"b": "c",
},
TxPool: &TxPoolConfig{
LifeTime: 5 * time.Second,
},
P2P: &P2PConfig{
MaxPeers: 10,
Discovery: &P2PDiscovery{
Expand Down
Loading

0 comments on commit 96ac8d1

Please sign in to comment.