You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
There seems to be a bit of code duplication and hidden dependency with some of the enums. These are maintained and managed in the chaostoolkit-lib project, but the choastoolkit project uses them for click options. This is totally fine however the value types are declared independently in both, this means if you change one you need to change the other, and since they are both separate projects there is a possibility that one might change before the other leading to issues.
I have implemented similar enum pattern in click in my own personal project and went with this method of just dynamically loading the choices from the enum at run time.
classHighlightingMode(Enum):
""" An Enum for managing the highlighting modes for edges """ALL="all"PRECEDING="preceding"SUCCESSOR="successor"
.....
.....
@click.option("--mode",type=click.Choice([e.valueforeinHighlightingMode]),default=HighlightingMode.PRECEDING.value,help="Select highlighting mode",)
The value in this pattern is it decouples the enum values being coded twice, once in the enum and once in the click command. This will ensure the options the user can choose are those which the enum currently provides. The above example is simplified, but i was using a similar pattern as CTK where the enum was implemented in the core package but then consumed in the cli package and this seems to scale well.
Personally I would drop the static from_string methods since they dont add any value that enum doesnt provide out of the box and cause scaling issues and duplication. so instead of having to maintain the if/elif structure, and call Strategy.from_string("after-method-only") you can just call Strategy("after-method-only") and achieve the same result, it will also automatically produce a value error for invalid strings.
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
-
There seems to be a bit of code duplication and hidden dependency with some of the enums. These are maintained and managed in the
chaostoolkit-lib
project, but thechoastoolkit
project uses them for click options. This is totally fine however the value types are declared independently in both, this means if you change one you need to change the other, and since they are both separate projects there is a possibility that one might change before the other leading to issues.CLI values https://github.com/chaostoolkit/chaostoolkit/blob/master/chaostoolkit/commands/run.py#L56
LIB values https://github.com/chaostoolkit/chaostoolkit-lib/blob/master/chaoslib/types.py#L93
I have implemented similar enum pattern in click in my own personal project and went with this method of just dynamically loading the choices from the enum at run time.
The value in this pattern is it decouples the enum values being coded twice, once in the enum and once in the click command. This will ensure the options the user can choose are those which the enum currently provides. The above example is simplified, but i was using a similar pattern as CTK where the enum was implemented in the core package but then consumed in the cli package and this seems to scale well.
Personally I would drop the static
from_string
methods since they dont add any value that enum doesnt provide out of the box and cause scaling issues and duplication. so instead of having to maintain the if/elif structure, and callStrategy.from_string("after-method-only")
you can just callStrategy("after-method-only")
and achieve the same result, it will also automatically produce a value error for invalid strings.Beta Was this translation helpful? Give feedback.
All reactions