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 loading of environment variables into the config #4479

Merged
merged 3 commits into from Mar 25, 2022

Conversation

pgjones
Copy link
Member

@pgjones pgjones commented Mar 8, 2022

This new method will pick out any environment variables with a certain
prefix and place them into the config named without the prefix. This
makes it easy to use environment variables to configure the app as is
now more popular than when Flask started.

The prefix should ensure that the environment isn't polluted and the
config isn't polluted by environment variables.

Checklist:

  • Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
  • Add or update relevant docs, in the docs folder and in code.
  • Add an entry in CHANGES.rst summarizing the change and linking to the issue.
  • Add .. versionchanged:: entries in any relevant code docs.
  • Run pre-commit hooks and fix any issues.
  • Run pytest and tox, no tests failed.

@@ -97,6 +97,31 @@ def from_envvar(self, variable_name: str, silent: bool = False) -> bool:
)
return self.from_pyfile(rv, silent=silent)

def from_prefixed_env(self, prefix: str = "FLASK_") -> bool:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that the prefix should include the _.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure here, although I agree dynaconf seems to have set the president of the _ being implied. I'd rather keep it explicit, but I'm happy either way.

@davidism
Copy link
Member

davidism commented Mar 15, 2022

I think only accepting strings is too big a limitation, given that it is possible to use types in all other config loading methods. Attempting to convert at least int, float, and bool would cover a lot of cases.

It would be nice to also accept list and dict, and maybe use __ double underscore for nested access.

Maybe this should take a convert_value function similar to how from_file takes a format loading function, so that users can override it further?

@pgjones
Copy link
Member Author

pgjones commented Mar 21, 2022

I agree with the conversion, I've followed dynaconf's convention (although simplified) and used json.loads (dynaconf uses toml). I've also made it configurable via a loads argument.

@@ -97,6 +105,44 @@ def from_envvar(self, variable_name: str, silent: bool = False) -> bool:
)
return self.from_pyfile(rv, silent=silent)

def from_prefixed_env(
self,
prefix: str = "FLASK_",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really sure what other separator except _ could be used, so I think this shouldn't include the separator. I tried export a.b=c and export a-b=c in ZSH and neither worked.

for raw_key, value in os.environ.items():
if raw_key.startswith(prefix):
key = raw_key[len(prefix) :] # Use removeprefix with Python 3.9
mapping[key] = loads(value)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the try/except Excetion should be here so that users don't have to write wrappers for any library they decide to use.

@davidism
Copy link
Member

I'm working on finalizing this, including the review I posted above, no need to push anything new.

@davidism
Copy link
Member

I've added support for nested dict access using the __ separator.

If an intermediate key doesn't exist, it will be initialized to an empty dict. This matches how top-level keys work (any config is added, it doesn't already have to be in the config dict), and allows specifying configuration for extensions without requiring calling init_app first, and without requiring init_app to add all possible config keys.

I did not add support for merging dicts or other nested access like lists or objects, or other advanced features. If even more control of config is needed, users should reach for Dynaconf or another dedicated config library.

@ThiefMaster
Copy link
Member

Do we really need/want this level of magic?

@davidism
Copy link
Member

I use it constantly at work, it is really handy for being able to configure deployments in Docker and other platform services. Nested access is nice for grouping config together rather than making it a flat namespace with long individual keys. Flask-SQLAlchemy, for example, has a dict as a config value for engine options.

pgjones and others added 2 commits March 25, 2022 11:54
This new method will pick out any environment variables with a certain
prefix and place them into the config named without the prefix. This
makes it easy to use environment variables to configure the app as is
now more popular than when Flask started.

The prefix should ensure that the environment isn't polluted and the
config isn't polluted by environment variables.

I've followed the dynaconf convention of trying to parse the
environment variable and then falling back to the raw value if parsing
fails.
* support nested dict access with "__" separator
* don't specify separator in prefix
* catch exceptions for any loads function
@davidism davidism added this to the 2.1.0 milestone Mar 25, 2022
@davidism davidism merged commit 2f5a2ab into pallets:main Mar 25, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants