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 — bpo-40816 Add AsyncContextDecorator class by heckad · Pull Request #20516 · python/cpython

Conversation

@heckad

@heckad heckad commented May 29, 2020 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

https://bugs.python.org/issue40816

Sorry, something went wrong.

Uh oh!

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

All reactions
@heckad
Add basic AsyncContextDecorator implementation
db99d4a
@heckad
heckad requested a review from 1st1 as a code owner May 29, 2020 15:00
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label May 29, 2020
@bedevere-bot bedevere-bot added the awaiting review label May 29, 2020
heckad and others added 2 commits May 29, 2020 18:19
@heckad
Fix
7d1243a
@blurb-it
📜🤖 Added by blurb_it.
20aeef8
eamanu
eamanu suggested changes May 30, 2020
View reviewed changes

@eamanu eamanu 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.

IMHO, we need test here, docs.. I thing this need more discussion?

Sorry, something went wrong.

Uh oh!

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

All reactions
@bedevere-bot bedevere-bot added awaiting core review and removed awaiting review labels May 30, 2020
@heckad

heckad commented May 30, 2020

Copy link
Copy Markdown
Contributor Author

Do you know you can write something like

@contextmanager
def safe_space(is_on=True):
    token = IS_SAFE_SPACE_VALUE.set(is_on)
    try:
        yield
    finally:
        IS_SAFE_SPACE_VALUE.reset(token)

And use it as

class MyClass:
    @safe_space()
    def safe_method(self):
        pass

but you can't write

@asynccontextmanager
async def async_safe_space(is_on=True):
    token = IS_SAFE_SPACE_VALUE.set(is_on)
    try:
        yield
    finally:
        IS_SAFE_SPACE_VALUE.reset(token)

because you get the error TypeError: '_AsyncGeneratorContextManager' object is not callable. This pull request makes '_AsyncGeneratorContextManager' callable. This is an implementation of lost functionality.

All reactions

Sorry, something went wrong.

Uh oh!

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

@heckad

heckad commented Jun 2, 2020

Copy link
Copy Markdown
Contributor Author

@1st1?

All reactions

Sorry, something went wrong.

Uh oh!

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

@njsmith

njsmith commented Jul 18, 2020 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

@belm0 asked me to look at this, but tbh I'm not real comfortable with adding new API surface to contextlib without input from the module owners, which I think is @ncoghlan and @1st1.

The idea seems straightforward enough. @eamanu is right though that it definitely needs docs and tests.

All reactions

Sorry, something went wrong.

Uh oh!

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

@heckad

heckad commented Jul 18, 2020

Copy link
Copy Markdown
Contributor Author

@njsmith, I will add tests if this pull request is interesting. I was just waiting for a response from @1st1.

All reactions

Sorry, something went wrong.

Uh oh!

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

@belm0

belm0 commented Jul 20, 2020

Copy link
Copy Markdown
Contributor

@heckad since core review attention is a scarce resource, I suggest having the PR ready to approve. Since the tests and docs can mostly be copied from ContextDecorator, not too much work?

This functionality is needed-- let's assume the maintainers will agree.

All reactions

Sorry, something went wrong.

Uh oh!

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

@heckad

heckad commented Jul 20, 2020

Copy link
Copy Markdown
Contributor Author

@belm0 How to improve the documentation?

All reactions

Sorry, something went wrong.

Uh oh!

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

@heckad
Add test
858a98f
belm0
belm0 reviewed Jul 20, 2020
View reviewed changes
Comment thread Lib/test/test_contextlib_async.py Resolved

Uh oh!

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

@belm0

belm0 commented Jul 20, 2020

Copy link
Copy Markdown
Contributor

@belm0 How to improve the documentation?

See context manager docs:

contextmanager() uses ContextDecorator so the context managers it creates can be used as decorators as well as in with statements. When used as a decorator, a new generator instance is implicitly created on each function call (this allows the otherwise “one-shot” context managers created by contextmanager() to meet the requirement that context managers support multiple invocations in order to be used as decorators).

(it would also need "Changed in version X.X" annotation)

Also see the ContextDecorator docs. Perhaps it's enough to say "Similar to ContextDecorator(), but works with async context managers" and give a short example.

https://docs.python.org/3/library/contextlib.html#contextlib.contextmanager
https://docs.python.org/3/library/contextlib.html#contextlib.ContextDecorator

All reactions

Sorry, something went wrong.

Uh oh!

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

@heckad
Fix test
1922773
1st1
1st1 reviewed Jul 22, 2020
View reviewed changes
Comment thread Lib/contextlib.py Resolved

Uh oh!

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

heckad added 2 commits July 22, 2020 11:02
@heckad
Fix function name
dcaa620
@heckad
Fix test
5e6cec7
1st1
1st1 reviewed Jul 23, 2020
View reviewed changes
Comment thread Lib/test/test_contextlib_async.py Resolved

Uh oh!

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

1st1
1st1 reviewed Jul 23, 2020
View reviewed changes
Comment thread Lib/test/test_contextlib_async.py Resolved

Uh oh!

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

1st1
1st1 reviewed Jul 23, 2020
View reviewed changes
Comment thread Lib/test/test_contextlib_async.py Resolved

Uh oh!

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

@heckad
Improve tests
a5fd929
1st1
1st1 approved these changes Jul 23, 2020
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Jul 23, 2020
1st1
1st1 requested changes Jul 23, 2020
View reviewed changes

@1st1 1st1 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.

Oh wait! The code/tests look great! But we also need to update the docs before we merge this.

Sorry, something went wrong.

Uh oh!

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

All reactions
@bedevere-bot bedevere-bot removed the awaiting merge label Jul 23, 2020
@bedevere-bot

bedevere-bot commented Jul 23, 2020

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

All reactions

Sorry, something went wrong.

Uh oh!

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

@bedevere-bot bedevere-bot added the awaiting changes label Jul 23, 2020
@heckad

heckad commented Jul 23, 2020

Copy link
Copy Markdown
Contributor Author

Need to add a version. Which version of python will be added this feature?

All reactions

Sorry, something went wrong.

Uh oh!

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

@heckad
Add doc
a1735c5
@heckad
heckad force-pushed the fix-issue-40816 branch from 3d7e06a to a1735c5 Compare July 23, 2020 19:44
1st1
1st1 reviewed Jul 23, 2020
View reviewed changes
Comment thread Doc/library/contextlib.rst Resolved

Uh oh!

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

@1st1

1st1 commented Jul 23, 2020

Copy link
Copy Markdown
Member

Need to add a version. Which version of python will be added this feature?

3.10.

All reactions

Sorry, something went wrong.

Uh oh!

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

@heckad

heckad commented Jul 23, 2020

Copy link
Copy Markdown
Contributor Author

Need to add a version. Which version of python will be added this feature?

3.10.

What about 3.9?

All reactions

Sorry, something went wrong.

Uh oh!

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

@1st1

1st1 commented Jul 23, 2020

Copy link
Copy Markdown
Member

We missed that train, unfortunately. 3.9 is frozen for new features.

All reactions

Sorry, something went wrong.

Uh oh!

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

@heckad
Improve doc
81df65c
1st1
1st1 reviewed Jul 27, 2020
View reviewed changes
Comment thread Doc/library/contextlib.rst Resolved

Uh oh!

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

belm0
belm0 reviewed Jul 27, 2020
View reviewed changes
Comment thread Doc/library/contextlib.rst Resolved

Uh oh!

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

heckad and others added 2 commits October 31, 2020 22:59
@heckad Yury Selivanov
Update Doc/library/contextlib.rst
1293c87
Co-authored-by: Yury Selivanov <[email protected]>
@heckad
Fix whitespaces
1bace39
@heckad

heckad commented Oct 31, 2020 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

@1st1, all changes were applied.

All reactions

Sorry, something went wrong.

Uh oh!

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

@heckad
heckad requested a review from 1st1 November 3, 2020 20:29
asvetlov
asvetlov approved these changes Nov 5, 2020
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting changes labels Nov 5, 2020
@asvetlov
asvetlov merged commit 178695b into python:master Nov 5, 2020
@bedevere-bot bedevere-bot removed the awaiting merge label Nov 5, 2020
@asvetlov

asvetlov commented Nov 5, 2020

Copy link
Copy Markdown
Contributor

Thanks @heckad

All reactions

Sorry, something went wrong.

Uh oh!

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

@heckad
heckad deleted the fix-issue-40816 branch November 5, 2020 09:00
shihai1991 added a commit to shihai1991/cpython that referenced this pull request Nov 6, 2020
@shihai1991
Merge remote-tracking branch 'origin/master' into bpo_41832
15e18cc
* origin/master:
  bpo-42179: Doc/tutorial: Remove mention of __cause__ (pythonGH-23162)
  bpo-26389: Allow passing an exception object in the traceback module (pythonGH-22610)
  bpo-42260: PyConfig_Read() only parses argv once (pythonGH-23168)
  bpo-42260: Add _PyConfig_FromDict() (pythonGH-23167)
  bpo-41877 Check for asert, aseert, assrt in mocks (pythonGH-23165)
  [docs] fix wrongly named AsyncContextDecorator (pythonGH-23164)
  bpo-42262: Add Py_NewRef() and Py_XNewRef() (pythonGH-23152)
  bpo-42266: Handle monkey-patching descriptors in LOAD_ATTR cache (pythonGH-23157)
  bpo-40816 Add AsyncContextDecorator class (pythonGH-20516)
shihai1991 added a commit to shihai1991/cpython that referenced this pull request Nov 7, 2020
@shihai1991
Merge remote-tracking branch 'origin/master' into bpo_41073_add_types…
2d72953
…lots1

* origin/master: (80 commits)
  bpo-42282: Fold constants inside named expressions (pythonGH-23190)
  bpo-41028: Doc: Move switchers to docsbuild-scripts. (pythonGH-20969)
  bpo-42133: update parts of the stdlib to fall back to `__spec__.loader` when `__loader__` is missing (python#22929)
  Remove outdated reference to pywin32 from platform module (pythonGH-22005)
  bpo-41832: PyType_FromModuleAndSpec() now accepts NULL tp_doc (pythonGH-23123)
  Minor grammar edits for the descriptor howto guide (GH-python#23175)
  bpo-42179: Doc/tutorial: Remove mention of __cause__ (pythonGH-23162)
  bpo-26389: Allow passing an exception object in the traceback module (pythonGH-22610)
  bpo-42260: PyConfig_Read() only parses argv once (pythonGH-23168)
  bpo-42260: Add _PyConfig_FromDict() (pythonGH-23167)
  bpo-41877 Check for asert, aseert, assrt in mocks (pythonGH-23165)
  [docs] fix wrongly named AsyncContextDecorator (pythonGH-23164)
  bpo-42262: Add Py_NewRef() and Py_XNewRef() (pythonGH-23152)
  bpo-42266: Handle monkey-patching descriptors in LOAD_ATTR cache (pythonGH-23157)
  bpo-40816 Add AsyncContextDecorator class (pythonGH-20516)
  bpo-42260: Add _PyInterpreterState_SetConfig() (pythonGH-23158)
  Disable peg generator tests when building with PGO (pythonGH-23141)
  bpo-1635741: _sqlite3 uses PyModule_AddObjectRef() (pythonGH-23148)
  bpo-1635741: Fix PyInit_pyexpat() error handling (pythonGH-22489)
  bpo-42260: Main init modify sys.flags in-place (pythonGH-23150)
  ...
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
@heckad Yury Selivanov @adorilson
bpo-40816 Add AsyncContextDecorator class (pythonGH-20516)
f2a1747
Co-authored-by: Yury Selivanov <[email protected]>
@lancelote lancelote mentioned this pull request May 29, 2021
Add contextlib.AsyncContextDecorator python/typeshed#5550
Merged
This was referenced Dec 23, 2021
bpo-38415 @asynccontextmanager as decorators like @contextmanager #16667
Merged
bpo-38415: Remove redundant AsyncContextDecorator.__call__ override from _AsyncG… #30233
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@asvetlov asvetlov asvetlov approved these changes
@1st1 1st1 Awaiting requested review from 1st1
+2 more reviewers
@belm0 belm0 belm0 left review comments
@eamanu eamanu eamanu requested changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

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

Assignees

No one assigned

Labels

None yet

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.

8 participants

@heckad @njsmith @belm0 @bedevere-bot @1st1 @asvetlov @eamanu @the-knights-who-say-ni
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.