ungleich-public/cdist

Fork 10

Replace all --param yes/no with boolean param! #313

New issue

Closed

opened 2021-11-20 15:24:08 +00:00 by ungleich-gitea · 6 comments

ungleich-gitea commented

2021-11-20 15:24:08 +00:00

Owner

Created by: telmich

Deprecate the older ones, keep for some versions and then switch to boolean?

Types:

__rvm_gemset

*Created by: telmich* Deprecate the older ones, keep for some versions and then switch to boolean? Types: __rvm_gemset

ungleich-gitea added the

cleanup

label 2021-11-20 15:24:08 +00:00

ungleich-gitea closed this issue

2021-11-20 15:24:08 +00:00

ungleich-gitea commented

2021-11-20 15:52:56 +00:00

Author

Owner

Created by: telmich

Already done! yeah!

*Created by: telmich* Already done! yeah!

ungleich-gitea commented

2021-11-20 15:52:57 +00:00

Author

Owner

Created by: telmich

Last Missing: __jail (blocker for 2.1)

*Created by: telmich* Last Missing: __jail (blocker for 2.1)

ungleich-gitea commented

2021-11-20 15:52:58 +00:00

Author

Owner

Created by: jdguffey

Err...Implement in types should be 2. Apparently I can't count. :/

*Created by: jdguffey* Err...Implement in types should be 2. Apparently I can't count. :/

ungleich-gitea commented

2021-11-20 15:53:00 +00:00

Author

Owner

Created by: jdguffey

Aha. I like this. Perhaps it could be solved in one of a couple ways (that would work alongside my previous suggestion).

Implement in cdist core

Since cdist already makes use of argparse and store_const, set const in boolean params to "true," then proceed as previously described in gencode-*.

emulator.py:

        for parameter in self.cdist_type.boolean_parameters:
            argument = "--" + parameter
-            parser.add_argument(argument, dest=parameter, action='store_const', const='')
+            parser.add_argument(argument, dest=parameter, action='store_const', const='true')

Implement in types

This one is harder but stays out of the core (I'd probably not prefer this method, but I'll include it for the sake of completeness):

if [ -f "$__object/parameter/blah" ]; then
   blah=$(cat "$__object/parameter/blah")
   [ -n "$blah" ] || blah="true"
fi
...

Eh?

*Created by: jdguffey* Aha. I like this. Perhaps it could be solved in one of a couple ways (that would work alongside my previous suggestion). 1. Implement in cdist core Since cdist already makes use of argparse and store_const, set const in boolean params to "true," then proceed as previously described in gencode-*. emulator.py: ``` for parameter in self.cdist_type.boolean_parameters: argument = "--" + parameter - parser.add_argument(argument, dest=parameter, action='store_const', const='') + parser.add_argument(argument, dest=parameter, action='store_const', const='true') ``` 1. Implement in types This one is harder but stays out of the core (I'd probably not prefer this method, but I'll include it for the sake of completeness): ``` if [ -f "$__object/parameter/blah" ]; then blah=$(cat "$__object/parameter/blah") [ -n "$blah" ] || blah="true" fi ... ``` Eh?

ungleich-gitea commented

2021-11-20 15:53:01 +00:00

Author

Owner

Created by: telmich

Good morning Jake,

Jake Guffey [Wed, Sep 12, 2012 at 06:32:55PM -0700]:

What if, rather than forcing everything to always be true/false (which is, of course, a legitimate option), we refactor types to allow all yes/no/true/false parameters to be (yes|true|1)|(no|false|0), like so many configuration file syntaxes in *NIX-land? Then, the user could choose whatever format made the most sense to him, while it would mean the same thing to cdist types on the back end? Something as simple as
if [ "$param" = "yes" -o "$param" = "true" -o "$param" -eq "1" ]; then
...
fi
or
case "$param" in
   "yes"|"true"|"1")
      ...
      ;;
esac
This also has the added benefit of not breaking pre-existing manifests that use yes/no rather than true/false without refactoring of each manifest.

Just my $0.02. Thoughts?

Good point - especially not breaking the user is a great idea.
Though what I was thinking about a different syntax, which may not be
usable with the current setup:

__directory /my/id --parents --mode 0700

As the parent parameter being present/absent, this is already enough to
distinguish betweeen true/false.

This has been added to cdist in version 2.0.8 already and the included
types should use this to encourage users to use this standardised way as well.

You could then check if it in the type is present with

if [ -f $__object/parameter/parents ]; ...

PGP key: 7ED9 F7D3 6B10 81D7 0EC5 5C09 D7DC C8E4 3187 7DF0

*Created by: telmich* Good morning Jake, Jake Guffey [Wed, Sep 12, 2012 at 06:32:55PM -0700]: > What if, rather than forcing everything to always be true/false (which is, of course, a legitimate option), we refactor types to allow all yes/no/true/false parameters to be (yes|true|1)|(no|false|0), like so many configuration file syntaxes in *NIX-land? Then, the user could choose whatever format made the most sense to him, while it would mean the same thing to cdist types on the back end? Something as simple as > > ``` > if [ "$param" = "yes" -o "$param" = "true" -o "$param" -eq "1" ]; then > ... > fi > ``` > > or > > ``` > case "$param" in > "yes"|"true"|"1") > ... > ;; > esac > ``` > > This also has the added benefit of not breaking pre-existing manifests that use yes/no rather than true/false without refactoring of each manifest. > > Just my $0.02. Thoughts? Good point - especially _not_ breaking the user is a great idea. Though what I was thinking about a different syntax, which may not be usable with the current setup: __directory /my/id --parents --mode 0700 As the parent parameter being present/absent, this is already enough to distinguish betweeen true/false. This has been added to cdist in version 2.0.8 already and the included types should use this to encourage users to use this standardised way as well. You could then check if it in the type is present with if [ -f $__object/parameter/parents ]; ... ## PGP key: 7ED9 F7D3 6B10 81D7 0EC5 5C09 D7DC C8E4 3187 7DF0

ungleich-gitea commented

2021-11-20 15:53:02 +00:00

Author

Owner

Created by: jdguffey

What if, rather than forcing everything to always be true/false (which is, of course, a legitimate option), we refactor types to allow all yes/no/true/false parameters to be (yes|true|1)|(no|false|0), like so many configuration file syntaxes in *NIX-land? Then, the user could choose whatever format made the most sense to him, while it would mean the same thing to cdist types on the back end? Something as simple as

if [ "$param" = "yes" -o "$param" = "true" -o "$param" -eq "1" ]; then
...
fi

case "$param" in
   "yes"|"true"|"1")
      ...
      ;;
esac

This also has the added benefit of not breaking pre-existing manifests that use yes/no rather than true/false without refactoring of each manifest.

Just my $0.02. Thoughts?

*Created by: jdguffey* What if, rather than forcing everything to always be true/false (which is, of course, a legitimate option), we refactor types to allow all yes/no/true/false parameters to be (yes|true|1)|(no|false|0), like so many configuration file syntaxes in *NIX-land? Then, the user could choose whatever format made the most sense to him, while it would mean the same thing to cdist types on the back end? Something as simple as ``` if [ "$param" = "yes" -o "$param" = "true" -o "$param" -eq "1" ]; then ... fi ``` or ``` case "$param" in "yes"|"true"|"1") ... ;; esac ``` This also has the added benefit of not breaking pre-existing manifests that use yes/no rather than true/false without refactoring of each manifest. Just my $0.02. Thoughts?