Skip to content

internal/cli: use config file for flags in new-cli, builder/files: update defaults#457

Merged
pratikspatil024 merged 31 commits into
developfrom
pratik/pos-586
Jul 27, 2022
Merged

internal/cli: use config file for flags in new-cli, builder/files: update defaults#457
pratikspatil024 merged 31 commits into
developfrom
pratik/pos-586

Conversation

@pratikspatil024

@pratikspatil024 pratikspatil024 commented Jul 15, 2022

Copy link
Copy Markdown
Member

This PR does the following changes

  1. Adds support for toml configuration files.
  2. Removed support for multiple configuration files.
  3. Adds a new sub command in the new-cli named dumpconfig which exports the flags provided to it while running in a toml format (for now). This commands works in the exact same way as in geth. Usage: bor dumpconfig <flags>.
  4. Use string splice and trim function from old cli for better results.
  5. Updates the bor.service file and moved the flags to the config.toml file.
  6. Updated the merge logic in command.go, and restructured the Run() function by creating a new function to extract values from the argument.

TODO's before merging

  • Discuss on the tag names for flags
  • Fix testcase (config_legacy_test.go)
  • Handel decoding of *big.Int type fields (MaxPrice, IgnorePrice, and GasPrice), and have a look at time.Duration type fields (TxPool.LifeTime, TxPool.Rejournal, and Cache.Rejournal) (It's working fine, but we still need to handle the dumpconfig output)
  • Test the working of -config flag
  • Unit test for flag parsing of each type (change assert to require).

Jira tickets:
POS-586: Move flags from CLI to config file

POS-651: Provide a default config file

POS-635: Add dumpconfig in bor

@manav2401 manav2401 changed the title Move flags from CLI to config file internal/cli: use config file for flags in new-cli, builder/files: update defaults Jul 15, 2022
@codecov-commenter

codecov-commenter commented Jul 24, 2022

Copy link
Copy Markdown

Codecov Report

Merging #457 (d1c1b18) into develop (c446937) will increase coverage by 0.66%.
The diff coverage is 18.98%.

@@             Coverage Diff             @@
##           develop     #457      +/-   ##
===========================================
+ Coverage    56.30%   56.96%   +0.66%     
===========================================
  Files          603      607       +4     
  Lines        69529    69899     +370     
===========================================
+ Hits         39149    39820     +671     
+ Misses       27012    26684     -328     
- Partials      3368     3395      +27     
Impacted Files Coverage Δ
internal/cli/command.go 6.17% <0.00%> (-0.18%) ⬇️
internal/cli/dumpconfig.go 0.00% <0.00%> (ø)
internal/cli/server/flags.go 100.00% <ø> (+100.00%) ⬆️
internal/cli/server/command.go 6.77% <15.00%> (+6.77%) ⬆️
internal/cli/server/config_legacy.go 33.33% <36.36%> (-44.45%) ⬇️
internal/cli/server/config.go 73.48% <60.00%> (+0.50%) ⬆️
internal/cli/flagset/flagset.go 36.79% <71.42%> (+2.13%) ⬆️
accounts/keystore/watch.go 95.34% <0.00%> (-4.66%) ⬇️
p2p/discover/table.go 82.48% <0.00%> (-3.65%) ⬇️
p2p/enode/iter.go 88.34% <0.00%> (-2.92%) ⬇️
... and 33 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c446937...d1c1b18. Read the comment docs.

@pratikspatil024 pratikspatil024 marked this pull request as ready for review July 25, 2022 08:50
@pratikspatil024 pratikspatil024 requested review from JekaMas and cffls July 25, 2022 09:55
Comment thread builder/files/bor.service Outdated
Comment thread builder/files/config.toml Outdated

@cffls cffls left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@manav2401 manav2401 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Great work 🚀

@pratikspatil024 pratikspatil024 merged commit 71980bf into develop Jul 27, 2022
@pratikspatil024 pratikspatil024 deleted the pratik/pos-586 branch July 27, 2022 06:58
cffls pushed a commit to cffls/bor that referenced this pull request Jul 30, 2022
…date defaults (0xPolygon#457)

* updated simple.json and simple.hcl

* added annotations for developer and grpc block

* added toml tags and simple.toml file

* added support for toml config files

* updated simple files toml, hcl, json

* added config.toml in builder/files and updated bor.service

* add dumpconfig command in cli for exporting configs

* update docs

* updated .goreleaser.yml (POS-651)

* changed --config to -config

* added test config for tests and fixed lint errors

* made fields of type big.int and time.Duration private, removed merge from dumpconfig, setting up default values to the Raw fields in dumpconfig, and fixed one lint error

* fixed lint errors (strange)

* lint fix

* private no-more, using '-' in name tags to ignore

* updated name tags, made c.configFile as a stringFlag (only one config file supported) and updated the merge logic in command.go -> Run()

* fix: set method for big.Int flags, added a TODO

* handeled bigInt and timeDuration type, bug fix in config_legacy, lint fix

* updated flags, consistent across flags.go and config.go

* fixed config test and updated test hcl, json config files

* updated config legacy test

* added test to check values of cmd flags, restructured Run in command.go, linter fix

* fix linters

* lint again

* changed 2 flags and assert -> require

* changed the 2 flags back

* updated correct congig.toml path and made mainnet default

* updated config.toml with new flags

* added sample config (toml) file

* removed sample-config.toml and added it in docs/config.md

Co-authored-by: Manav Darji <manavdarji.india@gmail.com>
cffls pushed a commit that referenced this pull request Aug 1, 2022
…date defaults (#457)

* updated simple.json and simple.hcl

* added annotations for developer and grpc block

* added toml tags and simple.toml file

* added support for toml config files

* updated simple files toml, hcl, json

* added config.toml in builder/files and updated bor.service

* add dumpconfig command in cli for exporting configs

* update docs

* updated .goreleaser.yml (POS-651)

* changed --config to -config

* added test config for tests and fixed lint errors

* made fields of type big.int and time.Duration private, removed merge from dumpconfig, setting up default values to the Raw fields in dumpconfig, and fixed one lint error

* fixed lint errors (strange)

* lint fix

* private no-more, using '-' in name tags to ignore

* updated name tags, made c.configFile as a stringFlag (only one config file supported) and updated the merge logic in command.go -> Run()

* fix: set method for big.Int flags, added a TODO

* handeled bigInt and timeDuration type, bug fix in config_legacy, lint fix

* updated flags, consistent across flags.go and config.go

* fixed config test and updated test hcl, json config files

* updated config legacy test

* added test to check values of cmd flags, restructured Run in command.go, linter fix

* fix linters

* lint again

* changed 2 flags and assert -> require

* changed the 2 flags back

* updated correct congig.toml path and made mainnet default

* updated config.toml with new flags

* added sample config (toml) file

* removed sample-config.toml and added it in docs/config.md

Co-authored-by: Manav Darji <manavdarji.india@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.

5 participants