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

Delegate chunked encoding handling to urllib #4958

Closed
wants to merge 1 commit into from

Conversation

leamingrad
Copy link

This MR removes the custom handling of sending chunked HTTP requests from requests to delegate to the equivalent logic in urllib3 (https://github.com/urllib3/urllib3/blob/4325867d1ae0d139a11c8689c2d2a5ba2c666c83/src/urllib3/connectionpool.py#L351).

As far as I can tell this has a couple of advantages:

  • Remove code that is duplicated between the two libraries
  • Allow fixes for outstanding issues in this area to be made in urllib3 so that users of both libraries benefit (I noticed 404 responses for chunk-encoded requests may not be received #4445 when having a quick trawl of the issues but I guess there are more)
  • All requests are now logged by the urllib3 debug logger (which was the issue I was originally trying to diagnose)

@leamingrad
Copy link
Author

Quick note on testing that I forgot to put in the description: I haven't added any specific tests to cover this feature, but I think that the code is covered by existing tests from what I can see - if there are extras that would be helpful I am happy to add them.

@codecov-io
Copy link

codecov-io commented Feb 3, 2019

Codecov Report

Merging #4958 into master will decrease coverage by 0.23%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4958      +/-   ##
==========================================
- Coverage   66.89%   66.66%   -0.24%     
==========================================
  Files          15       15              
  Lines        1577     1554      -23     
==========================================
- Hits         1055     1036      -19     
+ Misses        522      518       -4
Impacted Files Coverage Δ
requests/adapters.py 68.84% <100%> (-1.43%) ⬇️

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 64bde65...5ac5caa. Read the comment docs.

@leamingrad leamingrad closed this Mar 17, 2020
@leamingrad leamingrad deleted the delegate_chunked_requests branch March 17, 2020 09:50
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 29, 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.

None yet

2 participants