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 support for case-insensitive environment variables on Windows #49

Closed
wants to merge 1 commit into from

Conversation

barneygale
Copy link

Ensure variable lookups hit os.environ itself, rather than a dict copy

Fixes #47

Ensure variable lookups hit `os.environ` itself, rather than a dict copy

Fixes thesimj#47
@barneygale
Copy link
Author

Note that ChainMap was added in Python 3.3, but the metadata for this package suggests it still supports Python 2.7. I wonder if dropping Python 2 support in EnvYAML would be advisable?

@barneygale
Copy link
Author

Hey @thesimj, sorry to bother, would you be willing to review this and put out a new release if it looks good? Thank you!

@thesimj
Copy link
Owner

thesimj commented Aug 19, 2023

Hey @barneygale thanks for your contribution and commitment, but I think inserting case-insensitive environment variables could be dangerous for users. This could lead to unpredictable behaviors in env vars settings and other things we need to drop support for Python 2.7 (and we should drop it at some point).

@thesimj thesimj closed this Aug 19, 2023
@barneygale
Copy link
Author

Do you consider os.environ dangerous and unpredictable? :/

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.

lowercase environment will cause error
2 participants