-
Notifications
You must be signed in to change notification settings - Fork 106
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
Fix support for connection Upgrade and CONNECT when some data in the stream has been read. #882
Conversation
I have created a draft pull request to begin the work! My understanding is that the following work needs to be done...
@tomchristie I would appreciate any advice you can give me! |
Ah fantastic thanks, shall we start with a test?...
httpcore/tests/_async/test_http11.py Line 238 in 79fa6bf
I think the next step after that will be...
And then...
|
Thank you for your guidance 🙏 I'll give it a try. |
Work completed, I marked PR as ready. Could you please review 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.
This one is a fantastic fix, thanks. ✨
Minor suggestion wrt. the tests.
Later follow-ups we can consider...
- Should all HTTP requests include this behaviour or only
Upgrade
/CONNECT
? - Should all HTTP requests return
network_stream
or onlyUpgrade
/CONNECT
? - What might a nice
__repr__
be here?
(I'm just jotting these down here for reference)
Upgrade
and CONNECT
.Co-authored-by: Tom Christie <[email protected]>
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.
LGTM, Thank You!
Co-authored-by: T-256 <[email protected]>
Nice work. |
Thank you for your approval 🚀 It is a pleasure to contribute to this project. |
Summary
Upgrade
andCONNECT
. #872Checklist