Sitelet https://web.archive.org/web/20260624071615/https://github.com/python/cpython/pull/99899
Skip to content

gh-99714: asyncio - add shield_scope context manager and shielded decorator#99899

Closed
whatamithinking wants to merge 5 commits into
python:mainfrom
whatamithinking:fix-issue-99714
Closed

gh-99714: asyncio - add shield_scope context manager and shielded decorator#99899
whatamithinking wants to merge 5 commits into
python:mainfrom
whatamithinking:fix-issue-99714

Conversation

@whatamithinking

@whatamithinking whatamithinking commented Nov 30, 2022 •

Copy link
Copy Markdown

@ghost

ghost commented Nov 30, 2022 •

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.
CLA signed

@whatamithinking whatamithinking changed the title Fix issue 99714 gh-99714: asyncio - add shield_scope context manager and shielded decorator Nov 30, 2022
Comment thread Lib/asyncio/tasks.py Outdated
Comment on lines +81 to +82
shield = object() # just for making sure shields closed in order
# if __enter__ and __exit__ called directly

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But who would do that? Let them suffer. Task().shields can just be an integer counter.

Comment thread Lib/asyncio/tasks.py Outdated
Comment on lines +94 to +101
def shielded(func):
"""Decorator to shield the call of a async function or method,
wrapping it in a shield_scope()."""
@functools.wraps(func)
async def _shielded(*args, **kwargs):
with shield_scope():
return await func(*args, **kwargs)
return _shielded

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we absolutely, truly need two ways to do it (a decorator and a context manager)? I'd start with just the context manager.

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this require any updates to/for TaskGroup?

@gvanrossum

Copy link
Copy Markdown
Member

It looks like there isn't enough interest to pursue this. Closing. (Respond on the issue if you disagree.)

@gvanrossum gvanrossum closed this May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants