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

Jump to bottom

bpo-38013: make async_generator_athrow object tolerant to throwing exceptions #16070

Merged
merged 3 commits into from Sep 17, 2019
Merged

bpo-38013: make async_generator_athrow object tolerant to throwing exceptions #16070

merged 3 commits into from Sep 17, 2019

Conversation

asvetlov
Copy link
Contributor

@asvetlov asvetlov commented Sep 12, 2019 •
edited by miss-islington

Even when the helper is not started yet.

This behavior follows conventional generator one.
There is no reason for async_generator_athrow to handle gen.throw() differently.

https://bugs.python.org/issue38013

Automerge-Triggered-By: @asvetlov

@asvetlov asvetlov requested a review from 1st1 as a code owner Sep 12, 2019
1st1
1st1 requested changes Sep 12, 2019
View changes
Copy link
Member

@1st1 1st1 left a comment

I don't think this is the right way, although it might be. I'll need some time to carefully review the implementation. Please don't merge this.

@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Sep 12, 2019

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

And if you don't make the requested changes, you will be poked with soft cushions!

@asvetlov
Copy link
Author

@asvetlov asvetlov commented Sep 12, 2019

Sure, I want to wait for your approval.
Feel free to ask me for more information if needed.
I spent several hours in debugging; now I have a feeling that I completely understand what's going on and why the fix is correct.

An alternative PR that fixes the problem is #16061 but I prefer fixing a gen helper over adding a trampoline coroutine which is safely cancellable.

@1st1
Copy link

@1st1 1st1 commented Sep 12, 2019

Hmmm,

>>> def a(): yield
...
>>> a().throw(Exception())
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 1, in a
Exception

Didn't know this is true (or this has changed since 3.5).

Yeah, your fix is probably correct, but I'll need to still spend some time on it just to make sure I fully understand the consequences.

@asvetlov
Copy link
Author

@asvetlov asvetlov commented Sep 12, 2019

I wrote the same code as you have in the snippet to prove my idea :)

1st1
1st1 approved these changes Sep 17, 2019
View changes
@asvetlov asvetlov added expert-asyncio needs backport to 3.7 needs backport to 3.8 🤖 automerge labels Sep 17, 2019
@miss-islington miss-islington merged commit c275312 into python:master Sep 17, 2019
4 checks passed
@miss-islington
Copy link

@miss-islington miss-islington commented Sep 17, 2019

Thanks @asvetlov for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖

@asvetlov asvetlov deleted the bpo-38013-2 branch Sep 17, 2019
@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Sep 17, 2019

GH-16231 is a backport of this pull request to the 3.8 branch.

@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Sep 17, 2019

GH-16232 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this issue Sep 17, 2019
@asvetlov @miss-islington
bpo-38013: make async_generator_athrow object tolerant to throwing ex…
94f485b 
…ceptions (pythonGH-16070)

Even when the helper is not started yet.

This behavior follows conventional generator one.
There is no reason for `async_generator_athrow` to handle `gen.throw()` differently.

https://bugs.python.org/issue38013
(cherry picked from commit c275312)

Co-authored-by: Andrew Svetlov <andrew.svetlov@gmail.com>
miss-islington added a commit that referenced this issue Sep 17, 2019
@miss-islington @asvetlov
bpo-38013: make async_generator_athrow object tolerant to throwing ex…
fc022f0 
…ceptions (GH-16070)

Even when the helper is not started yet.

This behavior follows conventional generator one.
There is no reason for `async_generator_athrow` to handle `gen.throw()` differently.

https://bugs.python.org/issue38013
(cherry picked from commit c275312)

Co-authored-by: Andrew Svetlov <andrew.svetlov@gmail.com>
miss-islington added a commit that referenced this issue Sep 17, 2019
@miss-islington @asvetlov
bpo-38013: make async_generator_athrow object tolerant to throwing ex…
3c1786f 
…ceptions (GH-16070)

Even when the helper is not started yet.

This behavior follows conventional generator one.
There is no reason for `async_generator_athrow` to handle `gen.throw()` differently.

https://bugs.python.org/issue38013
(cherry picked from commit c275312)

Co-authored-by: Andrew Svetlov <andrew.svetlov@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ammaraskar ammaraskar

@aeros aeros

@1st1 1st1

Assignees
No one assigned
Labels
CLA signed expert-asyncio 🤖 automerge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@asvetlov @bedevere-bot @1st1 @miss-islington @ammaraskar @aeros @the-knights-who-say-ni