GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-108951: document how to terminate an `asyncio.TaskGroup` by picnixz · Pull Request #123837 · python/cpython

Conversation

@picnixz

@picnixz picnixz commented Sep 8, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Credits to @sobolevn for the original implementation. I just reformulated a bit the way the tasks are created for the docs to make it a bit simpler (I think?)

  • Issue: Add an asyncio.TaskGroup.cancel method #108951

📚 Documentation preview 📚: https://cpython-previews--123837.org.readthedocs.build/

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@picnixz
picnixz requested review from 1st1, asvetlov, gvanrossum, kumaraditya303 and willingc as code owners September 8, 2024 11:39
@bedevere-app bedevere-app Bot added awaiting review docs Documentation in the Doc dir skip news labels Sep 8, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Sep 8, 2024
Add an asyncio.TaskGroup.cancel method #108951
Closed
sobolevn
sobolevn reviewed Sep 8, 2024
View reviewed changes

@sobolevn sobolevn left a comment

Copy link
Copy Markdown
Member

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.

Thank you for polishing this! I am still not sure if this is worth adding to the docs or not. But, this is, luckily, up to asyncio maintainers to decide :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@picnixz

picnixz commented Sep 8, 2024

Copy link
Copy Markdown
Member Author

Yeah, I feel the same because the example is a bit artificial. Your example with randomness was probably closer to what people expect in practice, namely tasks that complete in a non-deterministic order and maybe one of them would want to cancel the entire group, but this also means a non-deterministic output (which I don't really want to have in the docs...).

As for playing the devil's advocate for this case, I don't see how we can "badly" use the helper, except if one explicitly changes the CancellableTaskGroup implementation (devil's advocates are welcomed by the way)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Eclips4 Eclips4 added needs backport to 3.12 only security fixes needs backport to 3.13 only security fixes labels Sep 8, 2024
gvanrossum
gvanrossum reviewed Sep 8, 2024
View reviewed changes
Comment thread Doc/library/asyncio-task.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Doc/library/asyncio-task.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Doc/library/asyncio-task.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Doc/library/asyncio-task.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

@picnixz

picnixz commented Sep 8, 2024

Copy link
Copy Markdown
Member Author

Yeah, I feel the same because the example is a bit artificial

I knew it. When I was writing it I was like "hum... it's too long", so I wanted someone else's eyes, so thanks a lot for the comments Guido. I'll address them tomorrow

gvanrossum reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz
picnixz force-pushed the docs/asyncio-task-group-cancel-108951 branch from 76942ed to 7c709b4 Compare September 9, 2024 12:51
@picnixz
Document how to cancel an asynchronous task group.
b4dc0bb
@picnixz
picnixz force-pushed the docs/asyncio-task-group-cancel-108951 branch from 7c709b4 to b4dc0bb Compare September 9, 2024 12:52
@picnixz

picnixz commented Sep 9, 2024

Copy link
Copy Markdown
Member Author

I kept the output because not everyone wants to run the code (sometimes, they just want to see the output). If you feel that neither the output nor the intermediate printing jobs are needed, please tell me. However, I think it's nice for the users to see a standalone working example.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gvanrossum
gvanrossum reviewed Sep 9, 2024
View reviewed changes

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

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.

Almost there! It looks nice and simple now.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Doc/library/asyncio-task.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

picnixz
picnixz commented Sep 10, 2024
View reviewed changes

@picnixz picnixz left a comment

Copy link
Copy Markdown
Member Author

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.

A last bike-shedding question: would it make sense to also rename CancelTaskGroup into StopTaskGroup? strictly speaking, the exception serves as a signal to cancel/stop the task group (in the English sense) but we neither use CancelledError nor .cancel().

We would still use cancel in the docstring but to highlight the non-use of CancelledError, having a visually different name could be helpful for users (but it could also be confusing because they are close to each other...).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
picnixz
picnixz commented Sep 10, 2024
View reviewed changes
Comment thread Doc/library/asyncio-task.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

@picnixz
Update Doc/library/asyncio-task.rst
4e1c6bc
gvanrossum
gvanrossum reviewed Sep 10, 2024
View reviewed changes
Comment thread Doc/library/asyncio-task.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

willingc
willingc reviewed Sep 10, 2024
View reviewed changes

@willingc willingc left a comment

Copy link
Copy Markdown
Contributor

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.

Thanks! I've added some suggestions.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Doc/library/asyncio-task.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Doc/library/asyncio-task.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

@picnixz

picnixz commented Sep 11, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

Maybe it's because I'm not an English native / asyncio expert, but I feel a slight difference with "terminate", "cancel", "stop" and "abort":

  • Cancel is kind of generic and can be used to convene the intent of "stopping" whatever is going on. In other words, it can mean "stop", "terminate", "abort" even though it does not use the cancellation in the asyncio sense.

  • Stop hints me that I could perhaps resume it later (though you could say it would have been a pause/resume). (A bit like SIGSTOP/SIGCONT).

  • Abort hints me that it's something abrupt and "not good". Like "abort the mission" or SIGABRT. It's more for an unexpected event.

I will go for terminate because:

  • it terminates/ends/cancels/finishes something,
  • it can be thought as SIGTERM which, when emitted, can be caught for a graceful exit (which is the case here, we literally catch the exception), and
  • it could be thought as the signal for reaching a "terminal" state where you wouldn't continue (in contrast to "stop").
willingc reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz
address review
cf21e18
@picnixz

picnixz commented Sep 11, 2024

Copy link
Copy Markdown
Member Author

@willingc I took the liberty of rewording a bit the introductory paragraph. I quite like the While <some-unfortunate-thing>, we can <some-solution> construction.

willingc reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

willingc
willingc approved these changes Sep 11, 2024
View reviewed changes

@willingc willingc left a comment

Copy link
Copy Markdown
Contributor

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.

Thanks @picnixz. I appreciate your work on this. I think this is ready for merge, but I will give other folks a little time for reviewing.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

picnixz reacted with heart emoji
All reactions
  • ❤️ 1 reaction
@bedevere-app bedevere-app Bot removed the awaiting review label Sep 11, 2024
@bedevere-app bedevere-app Bot added the awaiting merge label Sep 11, 2024
gvanrossum
gvanrossum approved these changes Sep 11, 2024
View reviewed changes

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

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.

Excellent! I will merge.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

picnixz and willingc reacted with hooray emoji
All reactions
  • 🎉 2 reactions
@gvanrossum
gvanrossum merged commit ef05801 into python:main Sep 11, 2024
@miss-islington-app

miss-islington-app Bot commented Sep 11, 2024

Copy link
Copy Markdown

Thanks @picnixz for the PR, and @gvanrossum for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the awaiting merge label Sep 11, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 11, 2024
@picnixz @miss-islington
pythongh-108951: Document how to terminate an asyncio.TaskGroup (pyth…
f4447ea
…onGH-123837)

We don't want to add another API, since the recipe is straightforward and rarely needed.

The advantage is that we could backport this to the earliest Python version that has taskgroups (3.11, alas in security mode already, so we'll just do 3.12 and 3.13).
(cherry picked from commit ef05801)

Co-authored-by: Bénédikt Tran <[email protected]>
@bedevere-app

bedevere-app Bot commented Sep 11, 2024

Copy link
Copy Markdown

GH-123956 is a backport of this pull request to the 3.13 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the needs backport to 3.13 only security fixes label Sep 11, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 11, 2024
@picnixz @miss-islington
pythongh-108951: Document how to terminate an asyncio.TaskGroup (pyth…
003b942
…onGH-123837)

We don't want to add another API, since the recipe is straightforward and rarely needed.

The advantage is that we could backport this to the earliest Python version that has taskgroups (3.11, alas in security mode already, so we'll just do 3.12 and 3.13).
(cherry picked from commit ef05801)

Co-authored-by: Bénédikt Tran <[email protected]>
@bedevere-app

bedevere-app Bot commented Sep 11, 2024

Copy link
Copy Markdown

GH-123957 is a backport of this pull request to the 3.12 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the needs backport to 3.12 only security fixes label Sep 11, 2024
@gvanrossum

gvanrossum commented Sep 11, 2024

Copy link
Copy Markdown
Member

(Presumably the 3.13 backport will be held up by the upcoming release, unless the release manager deems doc PRs harmless. :-)

willingc reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz
picnixz deleted the docs/asyncio-task-group-cancel-108951 branch September 11, 2024 15:47
@picnixz picnixz changed the title gh-108951: document how to cancel an asyncio.TaskGroup gh-108951: document how to terminate an asyncio.TaskGroup Sep 11, 2024
Yhg1s pushed a commit that referenced this pull request Sep 24, 2024
@miss-islington @picnixz
[3.13] gh-108951: Document how to terminate an asyncio.TaskGroup (GH-…
951c7db
…123837) (#123956)

gh-108951: Document how to terminate an asyncio.TaskGroup (GH-123837)

We don't want to add another API, since the recipe is straightforward and rarely needed.

The advantage is that we could backport this to the earliest Python version that has taskgroups (3.11, alas in security mode already, so we'll just do 3.12 and 3.13).
(cherry picked from commit ef05801)

Co-authored-by: Bénédikt Tran <[email protected]>
kumaraditya303 pushed a commit that referenced this pull request Sep 25, 2024
@miss-islington @picnixz
[3.12] gh-108951: Document how to terminate an asyncio.TaskGroup (GH-…
13565f1
…123837) (#123957)

gh-108951: Document how to terminate an asyncio.TaskGroup (GH-123837)

We don't want to add another API, since the recipe is straightforward and rarely needed.

The advantage is that we could backport this to the earliest Python version that has taskgroups (3.11, alas in security mode already, so we'll just do 3.12 and 3.13).
(cherry picked from commit ef05801)

Co-authored-by: Bénédikt Tran <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sobolevn sobolevn sobolevn left review comments
@willingc willingc willingc approved these changes
@gvanrossum gvanrossum gvanrossum approved these changes
@1st1 1st1 Awaiting requested review from 1st1 1st1 is a code owner
@asvetlov asvetlov Awaiting requested review from asvetlov asvetlov is a code owner
@kumaraditya303 kumaraditya303 Awaiting requested review from kumaraditya303 kumaraditya303 is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

docs Documentation in the Doc dir skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@picnixz @gvanrossum @1st1 @willingc @sobolevn @Eclips4
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.