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

Postpone request decoding #91

Merged
merged 1 commit into from Oct 14, 2020
Merged

Postpone request decoding #91

merged 1 commit into from Oct 14, 2020

Conversation

lundberg
Copy link
Owner

@lundberg lundberg commented Oct 10, 2020

This PR aims to enhance the performance and speed of RESPX by postponing instantiation of httpx.Request to places actually needed and used.

TODO:

Fixes #90

@codecov-io
Copy link

codecov-io commented Oct 10, 2020

Codecov Report

Merging #91 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #91   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           11        11           
  Lines         1210      1214    +4     
  Branches        80        80           
=========================================
+ Hits          1210      1214    +4     
Impacted Files Coverage Δ
respx/models.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aaac1c3...9a2f85f. Read the comment docs.

@lundberg
Copy link
Owner Author

About postponing the instantiation of Request and Response for the call stats, I think it could be moved from the transport.record method to the Call class. Agree @SlavaSkvortsov ?

This would enforce us to record the raw httpcore request and response tuples in the stats magic mock, and then handle decode_request and decode_response when unpacking a call tuple or accessing it.

Code that should trigger and return decoded requests and responses:

request, response = respx.calls[-1]  # also iter calls should return decoded 

last_call = respx.calls.last
last_call.response
last_call.request

Also worth mentioning, the decoded response holds a reference to the decoded request, which votes for local call knowledge of already instantiated request. Meaning, if one accesses response, the request needs to be decoded, and if request is accessed and asserted after that, the request should not need to be instantiated again. Maybe always decode the response on call access and have the request "property" as a shortcut to response.request?

@SlavaSkvortsov
Copy link
Contributor

I'll take a look on it today

@lundberg lundberg merged commit a9479b8 into master Oct 14, 2020
@lundberg lundberg deleted the postpone-request-decoding branch October 14, 2020 08:41
lundberg added a commit that referenced this pull request Oct 15, 2020
Added
- Added `text`, `html` and `json` content shorthands to ResponseTemplate. (PR #82)
- Added `text`, `html` and `json` content shorthands to high level API. (PR #93)
- Added support to set `http_version` for a mocked response. (PR #82)
- Added support for mocking by lowercase http methods, thanks @lbillinghamtn. (PR #80)
- Added query `params` to align with HTTPX API, thanks @jocke-l. (PR #81)
- Easier API to get request/response from call stats, thanks @SlavaSkvortsov. (PR #85)
- Enhanced test to verify better content encoding by HTTPX. (PR #78)
- Added Python 3.9 to supported versions and test suite, thanks @jairhenrique. (PR #89)

Changed
- `ResponseTemplate.content` as proper getter, i.e. no resolve/encode to bytes. (PR #82)
- Enhanced headers by using HTTPX Response when encoding raw responses. (PR #82)
- Deprecated `respx.stats` in favour of `respx.calls`, thanks @SlavaSkvortsov. (PR #92)

Fixed
- Recorded requests in call stats are pre-read like the responses. (PR #86)
- Postponed request decoding for enhanced performance. (PR #91)
- Lazy call history for enhanced performance, thanks @SlavaSkvortsov. (PR #92)

Removed
- Removed auto setting the `Content-Type: text/plain` header. (PR #82)
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.

Big overhead when mocking 149 urls
3 participants