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

feat: add support for case insensitive env names #313

Conversation

jasonkuhrt
Copy link

@jasonkuhrt jasonkuhrt commented Nov 25, 2018

feat: add support for case insensitive env names

Change Summary

BaseSettings can now be configured to read from os.environ in a
case-insensitive way.

Related issue number

Arguably will close #277. Doesn't quite meet the exact idea as proposed there, however it achieves the end goal that motivated #277, I think.

Performance Changes

pydantic cares about performance, if there's any risk performance changed on this PR,
please run make benchmark-pydantic before and after the change:

  • before: pydantic best=32.086μs/iter avg=33.792μs/iter stdev=1.696μs/iter
  • after: pydantic best=31.856μs/iter avg=32.579μs/iter stdev=0.731μs/iter

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes
  • No performance deterioration (if applicable)
  • HISTORY.rst has been updated
    • if this is the first change since a release, please add a new section
    • include the issue number or this pull request number #<number>
    • if you're not a regular contributer please include your github username @whatever

@jasonkuhrt
Copy link
Author

I will complete the other TODOs once I know the PR has a good chance of being merged.

@codecov
Copy link

codecov bot commented Nov 25, 2018

Codecov Report

Merging #313 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #313   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          13     13           
  Lines        1703   1708    +5     
  Branches      321    323    +2     
=====================================
+ Hits         1703   1708    +5

pydantic/env_settings.py Outdated Show resolved Hide resolved
@samuelcolvin
Copy link
Member

Definitely happy to merge once complete.

@jasonkuhrt
Copy link
Author

@samuelcolvin thanks, ready for review again.

docs/examples/settings_case_insensitive.py Outdated Show resolved Hide resolved
pydantic/env_settings.py Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
if env_var:

env_name_ = env_name.lower() if self.__config__.case_insensitive else env_name
env_val = env_vars.get(env_name_, None)
Copy link
Author

Choose a reason for hiding this comment

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

I came to the conclusion that env_val was a better ident name now that we have env_vars above. env_var in context of env_vars sounds like its an item from the list. Buts its not. It is a value from the environ dict while env_vars is said environ dict.

If you prefer to not touch env_var ident name, I would suggest using a different name than env_vars above. Maybe normalized_environ?

@samuelcolvin
Copy link
Member

Looks great. Thank you very much.

@samuelcolvin samuelcolvin merged commit a0aa9e7 into pydantic:master Nov 26, 2018
@jasonkuhrt
Copy link
Author

Thanks for the amazingly prompt collaboration! 🙏

@jasonkuhrt jasonkuhrt deleted the feat/277/base-settings-support-for-case-insensitive-env-names branch November 26, 2018 15:56
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

Successfully merging this pull request may close these issues.

Allow configuring env letter case rules for settings parser
2 participants