Make file_from_cl command line argument and file_from_env environment variable optional
ajanon opened this issue · 2 comments
As of ConfZ
1.6.0
, it is not possible to set ConfZFileSource
sources at runtime through the command line or environment while still allowing the CL argument or environment variable to be empty or unset. I will use file_from_cl
mostly here, but the same behavior can be seen with file_from_env
.
Given the following sources:
CONFIG_SOURCES = ConfZFileSource(file_from_cl="--config-file")
ConfZ
will try to get the config from the command line, which works as expected if provided. However, if I would also like my Config to fall back to another source down the line, or even to hard-coded fallback values defined in the class itself, this does not work. ConfZ
now requires the argument to be passed to the application. Adding optional=True
does not make the command line argument optional, it only means that it will not fail if the file does not exist.
For instance, if I want to use the following config:
class Config(ConfZ):
CONFIG_SOURCES = [
ConfZFileSource(file_from_cl="--config-file", optional=True),
ConfZCLArgSource()
]
value: int
To start such an application without any config files (e.g., for testing purposes) and to pass all values in the command line, you need to call it with:
app --config-file "not_a_config.yml" --value 10
With not_a_config.yml
not existing on the disk.
This works fine, but is a bit awkward.
Calling the same application with app --value 10
fails with: Command-line argument '--config' not found.
For this use case, I would expect optional
to also make the command line argument optional.
This issue seems related to #23.
I am not sure what behavior would make sense here.
For my proposal, it seems most if not all changes are in file_loader.py
. Something like this should work (not tested):
@classmethod
def populate_config(cls, config: dict, confz_source: ConfZFileSource):
try:
file_path = cls._get_filename(confz_source)
except ConfZFileException as e:
if confz_source.optional:
return
raise e
file_format = cls._get_format(file_path, confz_source.format)
file_content = cls._read_file(
file_path, file_format, confz_source.encoding, confz_source.optional
)
cls.update_dict_recursively(config, file_content)
This is a quick hack though, maybe you would like something more robust and maintainable.
If however, this looks good enough to you, I can submit a PR for this feature.
I agree, the current bebaviour is unexpected, optional
should also apply to file_from_cl
and file_from_env
. I would welcome a solution / PR as you proposed, catching the error, as the alternative (in each specific case preventing the error if it is an optional source) is not really more maintainable / robust in my opinion.
One thing I think would make sense it so make it a bit more consistent by also catching the file read error outside the _read_file
method and removing the prevention of the error within the function.
So if you would find the time for a PR, this would be great!