Skip to content

Fix werkzeug environ type #1744

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

Closed
wants to merge 2 commits into from
Closed

Conversation

srittau
Copy link
Collaborator

@srittau srittau commented Nov 14, 2017

PEP 3333 explicitly calls for environ to be a built-in dict. Using a
Mapping will not only prevent the dict from being modified (which is
explicitly allowed by PEP 3333), it will also cause interaction
problems when the environment is passed to other WSGI handlers.

Also change the value type from object to Any for convenience. By
definition, the values can be anything and can't be type checked.
Using object instead of Any forces us to explicitly cast the value
whenever we access it.

PEP 3333 explicitly calls for environ to be a built-in dict. Using a
Mapping will not only prevent the dict from being modified (which is
explicitly allowed by PEP 3333), it will also cause interaction
problems when the environment is passed to other WSGI handlers.

Also change the value type from object to Any for convenience. By
definition, the values can be anything and can't be type checked.
Using object instead of Any forces us to explicitly cast the value
whenever we access it.
This matches the type in wsgiref.types.
@srittau
Copy link
Collaborator Author

srittau commented Nov 19, 2017

Just noticed that this needs to be redone when #1730 gets merged, since it won't apply. also, it probably makes sense to use WSGIEnvironment from #1745.

@srittau
Copy link
Collaborator Author

srittau commented Jan 18, 2018

Superseded by #1831

@srittau srittau closed this Jan 18, 2018
@srittau srittau deleted the werkzeug-environ branch January 18, 2018 16:15
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.

1 participant