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

Fix #1458 - Allow for custom parsing of environment variables via parse_env_var in Config object #4406

Merged
merged 13 commits into from Aug 22, 2022

Conversation

acmiyaguchi
Copy link
Contributor

@acmiyaguchi acmiyaguchi commented Aug 19, 2022

Change Summary

This PR adds an parse_env_var classmethod into the Config object to allow for custom parsing of the environment within Settings. This is useful when specifying a custom format in environment strings e.g. lists (1,2,3) or key-value pairs (a=1,b=2,c=3). String-encoded JSON is not ideal in certain situations, such as when the environment is specified inside of a YAML file (leading to string-escaped string-encoded JSON).

This is an alternative to #3977

Related issue number

fixes #1458

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • changes/<pull request or issue id>-<github username>.md file added describing change
    (see changes/README.md for details)
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

@acmiyaguchi
Copy link
Contributor Author

please review

Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

Otherwise, I think this looks great.

Please update.

pydantic/env_settings.py Outdated Show resolved Hide resolved
pydantic/env_settings.py Outdated Show resolved Hide resolved
pydantic/env_settings.py Show resolved Hide resolved
@classmethod
def parse_env_var(cls, field_name: str, raw_val: str) -> Any:
if field_name == 'numbers':
return parse_list(raw_val)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return parse_list(raw_val)
return [int(x) for x in s.split(',')]

then remove parse_list above, I think this is just as easy to read and more compact.

I don't think you need the strip since int(' 123 ') == 123

@samuelcolvin
Copy link
Member

For the sake of speed, I've made the alterations I suggested - AFAIK this is the last change before V1.10 beta is released. 🎉

LGTM, @PrettyWood @hramezani please review.

tests/test_settings.py Outdated Show resolved Hide resolved
@hramezani
Copy link
Member

This is great 👍

I just left a small comment other than that LGTM 🚀

@samuelcolvin samuelcolvin enabled auto-merge (squash) August 22, 2022 15:58
@samuelcolvin
Copy link
Member

Okay, I'm going to go for it. I want to get the beta out today.

@PrettyWood if you see anything important, we can create another pR.

@acmiyaguchi thanks so much for this.

@samuelcolvin samuelcolvin merged commit fe7c9da into pydantic:main Aug 22, 2022
@acmiyaguchi acmiyaguchi deleted the parse-env-var branch August 22, 2022 16:29
@acmiyaguchi
Copy link
Contributor Author

Thank you for the review and accepting the patch into v1.10 beta! I appreciate Pydantic and look forward to seeing it mature.

@samuelcolvin
Copy link
Member

Thanks.

Somehow the change for this feature didn't get included in the changelog for v1.10a1, but the feature is there. The change will be included in the changelog for the next (pre) release.

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

Successfully merging this pull request may close these issues.

Settings: Custom parsing of environment variables (non-JSON)
4 participants