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.)
|
This seems like a step in the right direction, but further changes will be necessary. |
Could you be more specific? What use case are you thinking of? |
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 |
|
Ideally I would have liked the |
|
@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. |
|
Could you also tag me in that PR? Trying to wrap my head around the issue. |
|
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