cli: allow to override consensus wallet path #3198
Conversation
cli/options/options.go
Outdated
| // RelativePath is a flag for commands that use node configuration and provides | ||
| // the path to the consensus wallet instead of reading it from the config file. | ||
| var RelativePath = cli.StringFlag{ | ||
| Name: "relative-path", |
There was a problem hiding this comment.
s/relative-path/consensus-wallet?
With the current implementation it may be an absolute path, and it's OK. And the CLI flag should be more common, because when we start a node with some neo-go node -t --relative-path ./smth.json it's not very clear what's the purpose of this --relative-path, especially when we have --config-path and --config-file options.
There was a problem hiding this comment.
I kept it to how Roman named it in #3179 (comment) but I agree that the current name isn't very descriptive.
There was a problem hiding this comment.
Sorry, I'm still not clear enough with this. "relative-path" should be added to any relative path in the config. You may have other wallets for other services for example. Or you may want to have a relative path for your DB. Overriding a single parameter is not very exciting.
There was a problem hiding this comment.
And in that sense it'd be nice to add the same flag to cli/vm/vm.go that uses config files as well.
The result is that any non-absolute path in the config is relative-path + the_value_from_the_config. I think it'd be more powerful this way.
There was a problem hiding this comment.
I'm going to be way too inefficient in figuring out where all those paths are and how they work so I think it's best to leave it up to you guys then.
cli/options/options.go
Outdated
| @@ -73,6 +73,13 @@ var ConfigFile = cli.StringFlag{ | |||
| Usage: "path to the node configuration file (overrides --config-path option)", | |||
| } | |||
|
|
|||
| // RelativePath is a flag for commands that use node configuration and provides | |||
| // the path to the consensus wallet instead of reading it from the config file. | |||
| var RelativePath = cli.StringFlag{ | |||
There was a problem hiding this comment.
s/RelativePath/ConsensusWalletPath?
cli/options/options.go
Outdated
| if err == nil && len(relativePath) != 0 { | ||
| cfg.ApplicationConfiguration.Consensus.UnlockWallet.Path = relativePath | ||
| } | ||
| return cfg, err |
There was a problem hiding this comment.
Move this part to the end of the function, it allows to avoid code duplication.
cli/options/options.go
Outdated
| // the path to the consensus wallet instead of reading it from the config file. | ||
| var RelativePath = cli.StringFlag{ | ||
| Name: "relative-path", | ||
| Usage: "path to the consensus wallet file (overrides path defined in the config)", |
There was a problem hiding this comment.
s/defined in the config/defined in the Consensus subsection of the node configuration file
|
I'm aware it should become a single commit in the end. I'll squash once I get the green light it all looks as intended |
Problem
close #3179
Solution
as per #3179 (comment)
I wasn't sure if it made sense to add
options.RelativePathhereneo-go/cli/vm/vm.go
Line 16 in 8ed6d97
so I left it out until told otherwise.