-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Make HTTPResponse.read1
close response when all data is read
#3235
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks! Congrats in getting your CPython fix in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for clarifying the behavior of read1()
to me! I only had one review comment then this LGTM:
This is not required because `_raw_read` is not called by `read1` when `amt` is 0, but it makes `_raw_read` more explicit.
@sethmlarson @pquentin can we merge this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine to me, I'm sad we're having to get so specific for http.client
but since we're working around a bug in that implementation it makes sense. Thanks Illia!
In #3216, we discovered that CPython's
http.client.HTTPResponse.read1
never closes the response after all data has been read and content length was known.This PR adds a fix for the issue to
_raw_read
near a similar old fix that closes a response under a different condition.I'll create a PR to fix this in CPython too most likely.