Skip to content

Support for default values for optional config #2

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

Closed
mavwolverine opened this issue May 12, 2022 · 5 comments
Closed

Support for default values for optional config #2

mavwolverine opened this issue May 12, 2022 · 5 comments

Comments

@mavwolverine
Copy link

Something along the lines of

config.postgres_port or 5432
config.postgres_host or 'localhost'

@mavwolverine
Copy link
Author

I guess I could still do config.values.get('pg_port', 5432), but a simpler syntax would be great.

@RobertoPrevato
Copy link
Member

RobertoPrevato commented May 14, 2022

Thanks for your feedback! I have some mixed feelings about this, because supporting something like:

config.postgres_port or 5432

presents this issue:

  • if a user forgets to set at all a certain setting, (and doesn't use the or ... part), the code returns None, which might fail later (more details below on what is the original philosophy of this library)

As a side note, even Łukasz Langa recently commented on a similar subject, stating that it's more Pythonic to throw exception if an attribute is missing, rather than returning T | None (https://twitter.com/llanga/status/1522922855335436289).
Of course there are exceptions even in the standard library: Regex match returns a Match object or None - so I take this with a grain of salt.


The original philosophy of this library is to enable composition (something I like in .NET Core and wanted to have also for my Python projects), like:

  1. first read default values from example-settings.yaml / json
  2. then read values from example-test-settings.yaml / json (for the test environment - using an env variable to define the environment)
  3. then - for example - read values from environmental variables
  4. then read values from an Azure Key Vault / AWS Key Management Service

In other words, if you wish to have defaults, the idea of this library is that you would configure them either in memory:

    builder = ConfigurationBuilder()
    
    builder.add_map({
        "postgres_port ": 5432,
        "postgres_host": "localhost"
    })
    
    builder.add_source(YAMLFile("./yaml_example_01.yaml"))

    config = builder.build()

or, more likely, in a base app settings file:

{
    "postgres_port": 5432,
    "postgres_host": "localhost"
}

I know by practice that this has the good quality that applications settings are more understandable: it's sufficient to open this base settings file to get an overview of what settings are used by the app, and what are their defaults.

In conclusion, as I am thinking while writing these sentences, I don't think that returning default None would be the best option in this case.

@RobertoPrevato
Copy link
Member

RobertoPrevato commented May 14, 2022

But I like your recommendation, we could add a method similar to what dict offers:

configuration.get("key", default_value)

RobertoPrevato added a commit that referenced this issue May 14, 2022
A possible fix for #2
@RobertoPrevato
Copy link
Member

What about something like this? 368e9a4

@mavwolverine
Copy link
Author

Yes, that works. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants