ungleich-public/cdist

Fork 10

--beta on the commandline does not seem to work #131

New issue

Closed

opened 2021-11-20 13:24:26 +00:00 by ungleich-gitea · 7 comments

ungleich-gitea commented

2021-11-20 13:24:26 +00:00

Owner

Created by: matthijskooijman

With 4.8.0:

./bin/cdist inventory --beta list
ERROR: cdist: 'inventory' command is beta, but beta is not enabled. If you want to use it please enable beta functionalities by using the -b/--beta command line flag or setting CDIST_BETA env var.

Printing parser_args (at https://github.com/ungleich/cdist/blob/4.8.0/cdist/argparse.py#L439) gives:

Namespace(beta=False, command='inventory', config_file=None, func=<bound method Inventory.commandline of <class 'cdist.inventory.Inventory'>>, has_all_tags=False, host=[], hostfile=None, inventory_dir=None, list_only_host=False, quiet=False, subcommand='list', tag=False, verbose=None)

It seems the problem is that the "beta" parser is listed as a parser of the "inventory" parser as well as of the "list" parser, and the latter seems to overwrite parser_args['beta'] with its default. This is confirmed by the fact that this does work:

 ./bin/cdist inventory list --beta

Not sure what the proper fix would be, though.

*Created by: matthijskooijman* With 4.8.0: ``` ./bin/cdist inventory --beta list ERROR: cdist: 'inventory' command is beta, but beta is not enabled. If you want to use it please enable beta functionalities by using the -b/--beta command line flag or setting CDIST_BETA env var. ``` Printing `parser_args` (at https://github.com/ungleich/cdist/blob/4.8.0/cdist/argparse.py#L439) gives: ``` Namespace(beta=False, command='inventory', config_file=None, func=<bound method Inventory.commandline of <class 'cdist.inventory.Inventory'>>, has_all_tags=False, host=[], hostfile=None, inventory_dir=None, list_only_host=False, quiet=False, subcommand='list', tag=False, verbose=None) ``` It seems the problem is that the "beta" parser is listed as a parser of the "inventory" parser as well as of the "list" parser, and the latter seems to overwrite `parser_args['beta']` with its default. This is confirmed by the fact that this does work: ``` ./bin/cdist inventory list --beta ``` Not sure what the proper fix would be, though.

ungleich-gitea closed this issue

2021-11-20 13:24:26 +00:00

ungleich-gitea commented

2021-11-20 15:42:03 +00:00

Author

Owner

Created by: matthijskooijman

Fix looks good to me and works, thanks!

*Created by: matthijskooijman* Fix looks good to me and works, thanks!

ungleich-gitea commented

2021-11-20 15:42:05 +00:00

Author

Owner

Created by: darko-poljak

@matthijskooijman Please test fix: 23292e5cad

*Created by: darko-poljak* @matthijskooijman Please test fix: https://github.com/ungleich/cdist/commit/23292e5cad0ce3fad0ed82342bfede3f3fb6c77f

ungleich-gitea commented

2021-11-20 15:42:06 +00:00

Author

Owner

Created by: matthijskooijman

Looks good for the --beta option, sounds like the right way to fix this. CDIST_BETA env doesn't work yet, though.

*Created by: matthijskooijman* Looks good for the --beta option, sounds like the right way to fix this. `CDIST_BETA` env doesn't work yet, though.

ungleich-gitea commented

2021-11-20 15:42:07 +00:00

Author

Owner

Created by: darko-poljak

Fixed argparse parsers' parent references so options are now fixed/more consistent.

*Created by: darko-poljak* Fixed argparse parsers' parent references so options are now fixed/more consistent.

ungleich-gitea commented

2021-11-20 15:42:09 +00:00

Author

Owner

Created by: darko-poljak

@matthijskooijman I think that the right way would be:

$ ./bin/cdist inventory -h
usage: cdist inventory [-h] {add-host,add-tag,del-host,del-tag,list} ...

optional arguments:
  -h, --help            show this help message and exit

Inventory commands:
  {add-host,add-tag,del-host,del-tag,list}

Get cdist at http://www.nico.schottelius.org/software/cdist/

instead of

$ ./bin/cdist inventory -h
usage: cdist inventory [-h] [-l LOGLEVEL] [-q] [-v] [-b] [-g CONFIG_FILE]
                       [-I INVENTORY_DIR]
                       {add-host,add-tag,del-host,del-tag,list} ...

optional arguments:
  -h, --help            show this help message and exit
  -l LOGLEVEL, --log-level LOGLEVEL
                        Set the specified verbosity level. The levels, in
                        order from the lowest to the highest, are: ERROR (-1),
                        WARNING (0), INFO (1), VERBOSE (2), DEBUG (3) TRACE (4
                        or higher). If used along with -v then -v increases
                        last set value and -l overwrites last set value.
  -q, --quiet           Quiet mode: disables logging, including WARNING and
                        ERROR.
  -v, --verbose         Increase the verbosity level. Every instance of -v
                        increments the verbosity level by one. Its default
                        value is 0 which includes ERROR and WARNING levels.
                        The levels, in order from the lowest to the highest,
                        are: ERROR (-1), WARNING (0), INFO (1), VERBOSE (2),
                        DEBUG (3) TRACE (4 or higher). If used along with -l
                        then -l overwrites last set value and -v increases
                        last set value.
  -b, --beta            Enable beta functionality.
  -g CONFIG_FILE, --config-file CONFIG_FILE
                        Use specified custom configuration file.
  -I INVENTORY_DIR, --inventory INVENTORY_DIR
                        Use specified custom inventory directory. Inventory
                        directory is set up by the following rules: if cdist
                        configuration resolves this value then specified
                        directory is used, if HOME env var is set then
                        ~/.cdist/inventory is used, otherwise distribution
                        inventory directory is used.

Inventory commands:
  {add-host,add-tag,del-host,del-tag,list}

Get cdist at http://www.nico.schottelius.org/software/cdist/

and then for inventory subcommands:

usage: cdist inventory list [-h] [-l LOGLEVEL] [-q] [-v] [-b] [-g CONFIG_FILE]
                            [-I INVENTORY_DIR] [-a] [-f HOSTFILE] [-H] [-t]
                            [host [host ...]]
...

cdist inventory is just a sub-command sugar for e.g. inventory-add-host subcommand,
it has no functionality on its own, without its sub-commands.

*Created by: darko-poljak* @matthijskooijman I think that the right way would be: ``` $ ./bin/cdist inventory -h usage: cdist inventory [-h] {add-host,add-tag,del-host,del-tag,list} ... optional arguments: -h, --help show this help message and exit Inventory commands: {add-host,add-tag,del-host,del-tag,list} Get cdist at http://www.nico.schottelius.org/software/cdist/ ``` instead of ``` $ ./bin/cdist inventory -h usage: cdist inventory [-h] [-l LOGLEVEL] [-q] [-v] [-b] [-g CONFIG_FILE] [-I INVENTORY_DIR] {add-host,add-tag,del-host,del-tag,list} ... optional arguments: -h, --help show this help message and exit -l LOGLEVEL, --log-level LOGLEVEL Set the specified verbosity level. The levels, in order from the lowest to the highest, are: ERROR (-1), WARNING (0), INFO (1), VERBOSE (2), DEBUG (3) TRACE (4 or higher). If used along with -v then -v increases last set value and -l overwrites last set value. -q, --quiet Quiet mode: disables logging, including WARNING and ERROR. -v, --verbose Increase the verbosity level. Every instance of -v increments the verbosity level by one. Its default value is 0 which includes ERROR and WARNING levels. The levels, in order from the lowest to the highest, are: ERROR (-1), WARNING (0), INFO (1), VERBOSE (2), DEBUG (3) TRACE (4 or higher). If used along with -l then -l overwrites last set value and -v increases last set value. -b, --beta Enable beta functionality. -g CONFIG_FILE, --config-file CONFIG_FILE Use specified custom configuration file. -I INVENTORY_DIR, --inventory INVENTORY_DIR Use specified custom inventory directory. Inventory directory is set up by the following rules: if cdist configuration resolves this value then specified directory is used, if HOME env var is set then ~/.cdist/inventory is used, otherwise distribution inventory directory is used. Inventory commands: {add-host,add-tag,del-host,del-tag,list} Get cdist at http://www.nico.schottelius.org/software/cdist/ ``` and then for inventory subcommands: ``` usage: cdist inventory list [-h] [-l LOGLEVEL] [-q] [-v] [-b] [-g CONFIG_FILE] [-I INVENTORY_DIR] [-a] [-f HOSTFILE] [-H] [-t] [host [host ...]] ... ``` `cdist inventory` is just a sub-command sugar for e.g. inventory-add-host subcommand, it has no functionality on its own, without its sub-commands.

ungleich-gitea commented

2021-11-20 15:42:10 +00:00

Author

Owner

Created by: darko-poljak

@matthijskooijman Hi! Thanks for analysis. I am not sure for above, if argparse supports this, I have to check.
And for CDIST_BETA and beta default part, I will check and fix.

*Created by: darko-poljak* @matthijskooijman Hi! Thanks for analysis. I am not sure for above, if argparse supports this, I have to check. And for CDIST_BETA and beta default part, I will check and fix.

ungleich-gitea commented

2021-11-20 15:42:12 +00:00

Author

Owner

Created by: matthijskooijman

It seems that this also breaks the CDIST_BETA env var. It is parsed properly, but then overwritten with the default False from the commandline here: https://github.com/ungleich/cdist/blob/4.8.0/cdist/configuration.py#L469

I would suspect that removing the default from the beta parser here would fix both problems. This would mean that no 'beta' config is present if it is not given anywhere, but this is already handled here: https://github.com/ungleich/cdist/blob/4.8.0/cdist/argparse.py#L60-L61

*Created by: matthijskooijman* It seems that this also breaks the `CDIST_BETA` env var. It is parsed properly, but then overwritten with the default False from the commandline here: https://github.com/ungleich/cdist/blob/4.8.0/cdist/configuration.py#L469 I would suspect that removing the default from the beta parser [here](https://github.com/ungleich/cdist/blob/4.8.0/cdist/argparse.py#L130) would fix both problems. This would mean that no 'beta' config is present if it is not given anywhere, but this is already handled here: https://github.com/ungleich/cdist/blob/4.8.0/cdist/argparse.py#L60-L61