-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
bpo-39386: Prevent double awaiting of async iterator #18081
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
bpo-39386: Prevent double awaiting of async iterator #18081
Conversation
@1st1 would be very nice if you can find time for the PR review. |
@njsmith I appreciate your review as well |
LGTM, Andrew. Thank you. |
Thanks @asvetlov for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8. |
GH-18086 is a backport of this pull request to the 3.8 branch. |
(cherry picked from commit a96e06d) Co-authored-by: Andrew Svetlov <andrew.svetlov@gmail.com>
(cherry picked from commit a96e06d) Co-authored-by: Andrew Svetlov <andrew.svetlov@gmail.com>
GH-18087 is a backport of this pull request to the 3.7 branch. |
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.
Would it be redundant to explicitly test asend()
as well?
For example:
def test_async_gen_await_asend_twice(self):
async def async_iterate():
yield 1
yield 2
async def run():
it = async_iterate()
nxt = it.asend(None)
await nxt
with self.assertRaisesRegex(
RuntimeError,
r"cannot reuse already awaited __anext__\(\)/asend\(\)"
):
await nxt
self.loop.run_until_complete(run())
This seems effectively covered by test_async_gen_await_anext_twice
, but I don't think it hurts to include both (especially since it's straightforward and a quick test).
Otherwise, LGTM.
Oh oops, looks like I was typing that just as it was being reviewed by Yury, never mind. |
@aeros sorry, I've pressed "Merge" button before reading your message. |
@asvetlov No worries! It was more of an afterthought than anything, and I don't think it will have a significant impact on the long-term stability if we don't include the explicit |
https://bugs.python.org/issue39386