Uh oh!
There was an error while loading. Please reload this page.
bpo-46771: Remove two controversial lines from Task.cancel() - #31623
Conversation
Also from the _asyncio C accelerator module, and adjust one test that the change caused to fail. For more discussion see the discussion starting here: #31394 (comment) (Basically, @asvetlov proposed to return False from cancel() when there is already a pending cancellation, and I went along, even though it wasn't necessary for the task group implementation, and @agronholm has come up with a counterexample that fails because of this change. So now I'm changing it back to the old semantics (but still bumping the counter) until we can have a proper discussion about this.)
agronholm
commented
Feb 28, 2022
This seems like a step in the right direction, but further changes will be necessary. |
gvanrossum
commented
Feb 28, 2022
Could you be more specific? What use case are you thinking of? |
asvetlov
left a comment
There was a problem hiding this comment.
I'm fine with this..uncancel() makes the change not necessary.
Deprecation of cancellation message clears my other objections.
Sorry, I'm not active this week. Hard events in my life :(
agronholm
commented
Feb 28, 2022
Unless more changes are made, old-style cancel scope-like context managers will distort the cancel counter. It should be reset to 0 when the task is made cancellable again (when the |
agronholm
commented
Feb 28, 2022
Ideally I would have liked the |
gvanrossum
commented
Feb 28, 2022
@agronholm Would you mind submitting a (draft) PR? I personally think that if we have a solution that will work great in the future, making the existing cancel scope implementations have to do a version check would be a small price to pay. |
Tinche
commented
Mar 1, 2022
Could you also tag me in that PR? Trying to wrap my head around the issue. |
agronholm
commented
Mar 1, 2022
I'm currently on a work trip but will try work on that in the next few days. |
Also from the _asyncio C accelerator module,
and adjust one test that the change caused to fail.
For more discussion see the discussion starting here:
#31394 (comment)
(Basically, @asvetlov proposed to return False from cancel()
when there is already a pending cancellation, and I went along,
even though it wasn't necessary for the task group implementation,
and @agronholm has come up with a counterexample that fails
because of this change. So now I'm changing it back to the old
semantics (but still bumping the counter) until we can have a
proper discussion about this.)
https://bugs.python.org/issue46771