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: superfluous deep env conflicts with non-dict model leaf #276

Merged
merged 1 commit into from
Apr 23, 2024

Conversation

diefans
Copy link
Contributor

@diefans diefans commented Apr 23, 2024

If a model defines a leaf as e.g. a string and the user or system sets an environment var with an additional env_nested_delimiter + suffix, explode_env_vars was treating the string as a dict (trying to add an item to it).

This fix checks if the target is indeed a dict.

In that regard, the whole explode_env_vars seems a bit wasteful, in terms of possible throwing away already computed values, if a deeper environment variable is processed earlier (which is possible, since the iteration order of os.environ seems unpredictable).

resolves #275

@diefans
Copy link
Contributor Author

diefans commented Apr 23, 2024

test setup seems broken for mac/python 3.8/3.9
image

@hramezani
Copy link
Member

test setup seems broken for mac/python 3.8/3.9 image

Got fixed in a4aadfd
Please reabase your PR

If a model defines a leaf as e.g. a string and the user or system sets
an environment var with an additional `env_nested_delimiter + suffix`,
`explode_env_vars` was treating the string as a `dict` (trying to add an
item to it).

This fix checks if the target is indeed a `dict`.

In that regard, the whole `explode_env_vars` seems a bit wasteful, in
terms of possible throwing away already computed values, if a deeper
environment variable is processed earlier (which is possible, since the
iteration order of `os.environ` seems unpredictable).

resolves pydantic#275
@diefans
Copy link
Contributor Author

diefans commented Apr 23, 2024

@hramezani

Please reabase your PR

done

@hramezani hramezani merged commit 2d2f94f into pydantic:main Apr 23, 2024
18 checks passed
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.

env source: superflous deep env conflicts with non-dict model
2 participants