tox-node
tox-node copied to clipboard
`config` should be an option
This issue proposes to remove config
subcommand in favour of --config
option.
Motivation
An english dictionary gives the following definition of the word “subcommand”:
(computing) A command that makes up part of a larger command. This command accepts additional subcommands as parameters.
In command line interfaces, subcommands are used to execute specific actions. Some examples of various subcommands:
cargo build --example=foo
git commit -m "Initial commit"
http get https://github.com/
At this moment, tox-node
uses config
subcommand for running a node with a configuration file, and command line options to run a node without a configuration file. The main drawback of this approach is that it violates the subcommand semantics: config
is not an action. Beside that, this approach provides no way to override the parameters specified in the configuration file.
The distinction between the confiuration file option and other options was created to avoid the problem of mixing the configuration file with configuration options. This issue proposes a different solution: configuration merging.
Detailed design
- Remove the
config
subcommand - Add the
--config
cli option instead - Implement merging cli options with options in an configuration file
The left merge of configurations
A configuration A is a set of pairs (N, V)
, such as if (N, V_1) ∈ A
and (N, V_2) ∈ A
, than V_1 = V_2
. The first element of the pair is called a name and the second is called a value. We will write configurations as {N_1 = V_1, N_2 = V_2}
.
We say that name n
is in A if there exists such v that (n, v) ∈ A
.
A symmetric difference of configurations A and B is a configuration that contains pairs from both sets with names only in A or only in B. Symbolically, this is can be written as A ∆ B = {(n, v) | ((n, v) ∈ A ∧ (n, _) ∉ B) ∨ ((n, v) ∈ B ∧ (n, _) ∉ A)}
.
A left merge of configurations A and B, written A < B, is defined as this:
-
A ∆ B ⊂ A < B
- If
(n, a) ∈ A
and(n, b) ∈ B
than(n, b) ∈ A < B
Proposition: a left merge is a monoid with empty configuration as the identity.
Indeed:
-
{} < A = A < {} = A
, since A ∆ {} = A -
({x = a} < {x = b}) < {x = c} = {x = a} < ({x = b} < {x = c}) = {x = c}
Merging the configuration file with cli options
The result of merging the configuration file with cli options is the left merge of configurations. In other words, cli options have a priority over the configuration file, so when a parameter is defined by both the configuration file and a cli option, the cli option overrides parameter specified in the configuration file.
This allows to use the same configuration file for different configurations of the tox node. For example, a user may change the used key file with --keys-file
option, without changing the configuration file.
Alternatives
- Deprecate the
config
subcommand instead of removing - Do not merge the configuration, instead return an error when the
--config
option is used with other options.
How do we teach this?
The README.md already describes config
to be an option instead of a subcommand.
Drawbacks
- The change breaks scripts that use
config
as a subcommand - Slight increase of cli code complexy because of configuration merging
The main drawback of this approach is that it violates the subcommand semantics: config is not an action.
It can be considered as action "read config and run node", when default action is "run node". Your example with cargo demonstrates exact same thing - cargo test
builds project first with cargo build --tests
and only then runs tests. git also has such commands, e.g. git pull
= git fetch
+ git merge
. So it's not very clear when one command ends and another begins.
Beside that, this approach provides no way to override the parameters specified in the configuration file.
Do we really need it? Also there is another feature that often present in unix utils: arguments that are supposed to be unique can be specified multiple times. This way later arguments override earlier ones. This allows to alias commands with arguments and then override these arguments running aliases. Also to be able to override flags there are reversed flags, like --arg
and --no-arg
. So how far should we go? I have an impression that all this is contrary to the clap's approach.
Proposition: a left merge is a monoid with empty configuration as the identity.
This might be confusing for lists. For example, we have bootstrap nodes list and one might expect that specifying node via CLI will add it to the list but not override the whole list of nodes specified in the config.
Drawbacks
- The change breaks scripts that use config as a subcommand
- Slight increase of cli code complexy because of configuration merging
There is one more major drawback - this way we won't be able to use clap's validation and default arguments declaration. We'll have to assume that all CLI parameters are optional because they can be specified in config. So we'll have to check each required parameter manually and if it's not present in both config and CLI we'll have to panic. That was the reason why we used subcommand in the first place.
So it's not very clear when one command ends and another begins.
Still there's a very clear generic action (test the project, pull from a remote), unlike config
.
There is one more major drawback - this way we won't be able to use clap's validation.
I see. Well, then it's probably easier to just return an error when --config
is used with other options.
Still there's a very clear generic action (test the project, pull from a remote), unlike config.
But what if it was called run-with-config
? Also can we name it with dashes, just like --config
argument?
I see. Well, then it's probably easier to just return an error when --config is used with other options.
This also messes with parsing a bit. --config
will have to conflict with all arguments that don't have default value. Also instead of require
we will have to use require_unless("--config")
(though we don't have much requires right now).
This allows to use the same configuration file for different configurations of the tox node. For example, a user may change the used key file with --keys-file option, without changing the configuration file.
This is clear for --keys-file
, how about --bootstrap-node
?
How about default values? --threads
= 1 if nothing is set.
Also can we name it with dashes, just like
--config
argument?
Sounds like a hack.
How about default values? --threads = 1 if nothing is set.
Defaults < configuration file < cli arguments.
Defaults < configuration file < cli arguments.
Why? When there could be an easy way to use only Defaults < configuration file
. Because simple logic > complicated logic.
Yes, I agree that this kind of logic is probably too complicated. So the most reasonable solution is indeed to forbid using --config
with other arguments.
This is possible with https://docs.rs/clap/2.32.0/clap/struct.Arg.html#method.conflicts_with_all , but we will have to list every arg in conflicts_with_all(&[arg1, arg2, arg3])
and add require_unless("--config")
for required args. Or ask clap-rs to add a method named unique
for such args =)
It will bloat the code, so we have to cover it with tests.
But what if it was called
run-with-config
?
How about that? or with-config
?