Open silvanmelchior opened 3 years ago
Not sure if this is off-topic but I will post it anyway: I noticed that when I have an int enum in my config file and want to override it using ConfZCLArgSource I get a mixup of data types. The value set using CLI is a string while enum expects to be initialized via int. I'm not sure if there is an easy fix to that problem.
Let's say this is my enum that is part of a ConfZ class.
class MyConfig(ConfZ):
class SignalingMethod(Enum):
AzureIotHub = 1
TcpSocket = 2
signaling_method: SignalingMethod = SignalingMethod.AzureIotHub
CONFIG_SOURCES = ConfZCLArgSource(prefix='conf_')
Now I would like to set the value using --conf_signaling_method 1
.
This results in the following error:
value is not a valid enumeration member; permitted: 1, 2 (type=type_error.enum; enum_values=[<SignalingMethod.AzureIotHub: 1>, <SignalingMethod.TcpSocket: 2>])
Hmm, I don't have the time to reproduce this right now, but looks like pydantic doesn't like the string value. In https://pydantic-docs.helpmanual.io/usage/types/#enums-and-choices, they use IntEnum
, maybe this helps? Otherwise one might need to see what ConfZ passes to pydantic (expected would be the string "1") and if pydantic can parse this.
So far, command line arguments are parsed in a very simple way, reading from sys.argv directly. This allows to deal with arbitrary white spacing and with quotes. However, an equal sign between attribute name and value (e.g. --my_config_value=3) is not supported because sys.argv does not automatically separate this.
This issue concerns both ConfZCLArgSource and ConfZFileSource.file_from_cl
So far, it is not clear how to get a more robust parsing. Libraries like argparse require to know the allowed parameters beforehand, which is not the case right now with the current architecture. It might require a complete architecture change which first analyses the config class and then reads the config sources, instead of reading the sources without knowing the config class and just passing the result to pydantic. On the other hand, such a change would also be necessary to e.g. support #12.