bpo-38242: Revert "bpo-36889: Merge asyncio streams (GH-13251)" by 1st1 · Pull Request #16482 · python/cpython (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation13 Commits1 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
99% of the work is done by @aeros167; I've just needed to add a few more fixes and couldn't push to his branch; hence the new PR.
Thanks for finishing this up Yury. Out of curiosity, what additional changes needed to be made?
Thanks for finishing this up Yury. Out of curiosity, what additional changes needed to be made?
.write() and .close() were still awaitable -- which technically was committed before new streams api. Still we want to revert streams to their 3.7 state.
| .. coroutinefunction:: start_server(client_connected_cb, host=None, \ |
|---|
| port=None, \*, loop=None, limit=2**16, \ |
| port=None, \*, loop=None, limit=None, \ |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The constant _DEFAULT_LIMIT (2**16) is still being used as the default for limit in start_server(), so this can be restored.
| port=None, \*, loop=None, limit=None, \ |
|---|
| port=None, \*, loop=None, limit=2**16, \ |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can do this in a separate PR if needed. This change is rather low priority in comparison to reverting the streaming API, and would not be a release blocker.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, although right now docs are the same as in 3.7. So I'm not sure it's needed tbh.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I'm not sure it's needed tbh.
Okay, I won't worry about it then, it's fairly trivial.
.write() and .close() were still awaitable -- which technically was committed before new streams api. Still we want to revert streams to their 3.7 state.
Ah, I wasn't aware we were also removing that change. I hadn't realized we were reverting fully back to the 3.7 API, I thought it was primarily just focused on 23b4b69. Now it makes a lot more sense why you wanted to remove _asyncio_internal.
@1st1: Please replace # with GH- in the commit message next time. Thanks!
Thanks @1st1 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖
Sorry, @1st1, I could not cleanly backport this to 3.8 due to a conflict.
Please backport using cherry_picker on command line.cherry_picker 6758e6e12a71ef5530146161881f88df1fa43382 3.8
1st1 added a commit to 1st1/cpython that referenced this pull request
1st1 added a commit that referenced this pull request
jacobneiltaylor pushed a commit to jacobneiltaylor/cpython that referenced this pull request