Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow file_from_env and file_from_cl to override file in FileSource? #79

Open
N0K0 opened this issue Aug 10, 2023 · 1 comment
Open
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@N0K0
Copy link

N0K0 commented Aug 10, 2023

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?

def _get_filename(cls, config_source: FileSource) -> Path:
if config_source.file is not None:
if isinstance(config_source.file, bytes):
raise FileException("Can not detect filename from type bytes")
file_path = Path(config_source.file)
elif config_source.file_from_env is not None:
if config_source.file_from_env not in os.environ:
raise FileException(
f"Environment variable '{config_source.file_from_env}' is not set."
)
file_path = Path(os.environ[config_source.file_from_env])
elif config_source.file_from_cl is not None:

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.

@silvanmelchior
Copy link
Collaborator

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 silvanmelchior added enhancement New feature or request good first issue Good for newcomers labels Aug 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants