-
Notifications
You must be signed in to change notification settings - Fork 6
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
Added support for direct initialization #23
Conversation
goodconf/__init__.py
Outdated
elif load: | ||
return self.load() | ||
|
||
_config_file: str | None = PrivateAttr() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This requires Pydantic 1.7, if this is not okay for you then I can work around this.
Codecov Report
@@ Coverage Diff @@
## main #23 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 9 9
Lines 437 482 +45
Branches 50 52 +2
=========================================
+ Hits 437 482 +45
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
6c667dc
to
19673f7
Compare
pyproject.toml
Outdated
@@ -21,7 +21,7 @@ classifiers = [ | |||
] | |||
dynamic = ["version"] | |||
dependencies = [ | |||
"pydantic>=1.0" | |||
"pydantic>=1.7,<2.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure 2.0 will be breaking for our use case? I'm hesitant to add the upper version restriction.
env_settings, | ||
file_config_settings_source, | ||
file_secret_settings, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you document the precedence here in the README?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just looked at the README and it already documents the precedence of env over files. I'd prefer not to document anything else currently and wait to see if pydantic 2 would require further changes. In the same sense I would not document initialization via __init__
now. Everyone using this library and knows pydantic can use it, but let's wait and see what pydantic 2 does before making this an official feature.
Will document the precedence. But just so there is no misunderstanding, for all intents and purposes the precedence stays as it was before. That is ENV wins over file.cfg and initialization wins over everything else (which wasn't possible before).
As for the pydantic version boundary: will remove it, we can always issue a new release if we find out that 2.0 is not compatible.
…On Wed, Jan 11, 2023, at 22:14, Peter Baumgartner wrote:
***@***.**** commented on this pull request.
In goodconf/__init__.py
<#23 (comment)>:
> @@ -94,34 +127,21 @@ def customise_sources(
file_secret_settings: SettingsSourceCallable,
) -> Tuple[SettingsSourceCallable, ...]:
"""Load environment variables before init"""
- return env_settings, init_settings, file_secret_settings
-
- def load(self, filename: str = None) -> None:
+ return (
+ init_settings,
+ env_settings,
+ file_config_settings_source,
+ file_secret_settings,
+ )
Could you document the precedence here in the README?
—
Reply to this email directly, view it on GitHub
<#23 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAT5C4BCBG6PUFFLWER5JDWR4PD5ANCNFSM6AAAAAATUSPLFU>.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
@apollo13 could you update |
This is a re-implementation of #14 without any backwards incompatible changes (aside from a pydantic version increase)