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

unsplit the wrapper mixins #2003

Merged
merged 1 commit into from
Jan 15, 2021
Merged

unsplit the wrapper mixins #2003

merged 1 commit into from
Jan 15, 2021

Conversation

davidism
Copy link
Member

  • base_request.py and base_response.py were renamed to request.py and response.py. BaseRequest became Request, and BaseResponse became Response. All mixin code was moved into the classes.
  • All mixins show a deprecation warning in __init__.
  • BaseRequest and BaseResponse can still be used in isinstance and issubclass, they will show a deprecation warning and actually check against Request and Response.
  • PlainRequest is deprecated in favor of passing shallow=True, since that's just as effective at preventing reading the input stream.
  • Rewrote the docstrings for Request and Response.
  • ETagResponseMixin provided an overridden freeze method, but it wasn't active on the Response class due to the inheritance order. It adds an ETag header if one is not already set. I kept/added this behavior during the merge, since it made about as much sense as setting the Content-Length header. I also deprecated the no_etag parameter (that wasn't visible in Response either), since I didn't see a clear reason to allow choosing this during freeze and didn't want to open up the door to more such parameters if freeze is updated in the future.
  • Merged JSONMixin and adapted it to be specific to request vs response behavior. Removed caching for response.get_json (Cache Param For get_json() Leading to Contradictory Behavior #1698) as it caused issues if the data changed. Removed special exception handling for response. For some reason the mixin was written to use json_module, even though only loads is called. I left it as-is because it avoids having to wrap staticmethod(json.loads), and having dumps as well might be useful in the future. The request and response now have json, get_json, and related methods by default.
  • Using git blame, git doesn't see that response.py was actually base_response.py, and so the history doesn't look very interesting. Using git blame -M -C will tell it to figure out that the file moved instead, so it correctly sees the history from base_response.py and the mixin modules (and wrappers.py further back in history). PyCharm can also do this by selecting the "Detect movements across files" option when right clicking in the annotation sidebar.

Checklist:

  • Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
  • Add or update relevant docs, in the docs folder and in code.
  • Add an entry in CHANGES.rst summarizing the change and linking to the issue.
  • Add .. versionchanged:: entries in any relevant code docs.
  • Run pre-commit hooks and fix any issues.
  • Run pytest and tox, no tests failed.

@davidism davidism added this to the 2.0.0 milestone Jan 15, 2021
@davidism davidism merged commit f3b0816 into master Jan 15, 2021
@davidism davidism deleted the unsplit-wrappers branch January 15, 2021 18:32
@davidism davidism mentioned this pull request Jan 15, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

unsplit the wrapper mixins Cache Param For get_json() Leading to Contradictory Behavior
1 participant