ConfZ icon indicating copy to clipboard operation
ConfZ copied to clipboard

Allow file_from_env and file_from_cl to override file in FileSource?

Open N0K0 opened this issue 2 years ago • 1 comments

Code:

class ConfigRoot(BaseConfig):  # type: ignore
    mail: dict[str, MailConfig]
    redis: RedisConfig
    CONFIG_SOURCES = FileSource(file="config.toml", file_from_env="CONFIG_FILE")

What I'm looking for is for a way to make it so that if CONFIG_FILE is not set, fallback to config.toml. Right now file is prioritized, then env, then cl. There should probably be a Value error thrown if more than one of the flags are truthy? As they can not work together?

https://github.com/Zuehlke/ConfZ/blob/ecd5b0bd9ec2667cb638fb2d16554a6908463068/confz/loaders/file_loader.py#L21-L32

For me, it seems the fix can be as simple as swapping the elif with if statements, and making from env and from cl mutually exclusive, for example? Or make the params order sensitive somehow?

Alternatives: Having multiple file sources all marked as optional seems like a bit of an iffy approach? As we end up with either a validation error or an empty object I need to check, instead some error regarding resolution.

Example:

class ConfigRoot(BaseConfig):  # type: ignore
    mail: dict[str, MailConfig]
    redis: RedisConfig
    CONFIG_SOURCES = [
        FileSource(file_from_env="CONFIG_FILE", optional=True),
        FileSource(file="config.toml", optional=True),
    ]

In this scenario, neither CONFIG_FILE is set and config.toml is available. We then end up with a pydantic validation error instead on any loading related error.

N0K0 avatar Aug 10 '23 20:08 N0K0

Thanks for the detailed description! I agree, it would be useful to have a "default value" for the config file path, if no env-var / cli-arg is provided.

I think it would be a bit more work than just swapping the elif with if; we end up with quite a bit of cases. But also not too complicated I guess.

If anyone suggests a PR, I'm happy to review it.

silvanmelchior avatar Aug 13 '23 11:08 silvanmelchior