[issue29302] add contextlib.AsyncExitStack

[miss-islington]

miss-islington  added the comment:


New changeset e5125f7b3b88d8d4814ed7bddbd4f34d24d763dd by Miss Islington (bot) 
in branch '3.8':
Add Ilya Kulakov to Misc/ACKS. (GH-17130)
https://github.com/python/cpython/commit/e5125f7b3b88d8d4814ed7bddbd4f34d24d763dd


--
nosy: +miss-islington

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[miss-islington]

Change by miss-islington :


--
pull_requests: +16649
pull_request: https://github.com/python/cpython/pull/17138

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Benjamin Peterson]

Benjamin Peterson  added the comment:


New changeset d6d6e2aa0249bb661541705335ddbb97a53d64c8 by Benjamin Peterson 
(Ilya Kulakov) in branch 'master':
Add Ilya Kulakov to Misc/ACKS. (GH-17130)
https://github.com/python/cpython/commit/d6d6e2aa0249bb661541705335ddbb97a53d64c8


--
nosy: +benjamin.peterson

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Ilya Kulakov]

Change by Ilya Kulakov :


--
pull_requests: +16640
pull_request: https://github.com/python/cpython/pull/17130

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Yury Selivanov]

Yury Selivanov  added the comment:

Thank you Alexander and Ilya!

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Yury Selivanov]

Yury Selivanov  added the comment:


New changeset 1aa094f74039cd20fdc7df56c68f6848c18ce4dd by Yury Selivanov (Ilya 
Kulakov) in branch 'master':
bpo-29302: Implement contextlib.AsyncExitStack. (#4790)
https://github.com/python/cpython/commit/1aa094f74039cd20fdc7df56c68f6848c18ce4dd


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Nick Coghlan]

Nick Coghlan  added the comment:

Explicitly noting some API design decisions from the review of 
https://github.com/python/cpython/pull/4790:

1. AsyncExitStack will define __aenter__ and __aexit__ but *not* __enter__ and 
__exit. This makes it unambiguous which form of with statement you need to be 
using for a given stack.
2. AsyncExitStack will use the same methods to add synchronous context managers 
as ExitStack does
3. The only common method with clearly distinct external behaviour will be 
stack.close(), which will be a coroutine in the AsyncExitStack case (the other 
common methods will all still be synchronous in AsyncExitStack)
4. AsyncExitStack will gain 3 new methods for working with async context 
managers:

- enter_async_context(cm): coroutine to enter an async context manager and adds 
its __aexit__ to the context stack if __aenter__ succeeds
- push_async_exit(cm_or_exit): push an exit callback directly onto the stack. 
The argument must either be an object implementing __aexit__, or else a 
callback with the same signature as __aexit__ (use stack.push() to register a 
synchronous CM or exit callback)
- push_async_callback(callback): push the given callback onto the stack. The 
callback should return an awaitable (use stack.callback() to register 
synchronous callbacks)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Cheryl Sabella]

Cheryl Sabella  added the comment:

Looks like AbstractAsyncContextManager is defined in issue #30241.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Ilya Kulakov]

Ilya Kulakov  added the comment:

Charyl, I made the PR.

Where is the AbstractAsyncContextManager? I see that typing.py references it, 
but there is no actual implementation.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Ilya Kulakov]

Change by Ilya Kulakov :


--
keywords: +patch
pull_requests: +4690
stage: needs patch -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Cheryl Sabella]

Change by Cheryl Sabella :


--
stage:  -> needs patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Cheryl Sabella]

Cheryl Sabella  added the comment:

Alexander,

Did you want to submit a PR for this?

--
nosy: +csabella

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[privatwolke]

Change by privatwolke :


--
nosy: +privatwolke

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Nathaniel Smith]

Changes by Nathaniel Smith :


--
nosy: +njs

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Alexander Mohr]

Alexander Mohr added the comment:

let me know if I need to do anything

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Nick Coghlan]

Nick Coghlan added the comment:

While it *may* be possible to do something simpler for test purposes where 
performance isn't a major concern, fully supporting type() level mocking 
basically requires bringing the equivalent of wrapt object proxies into the 
standard library: 
https://wrapt.readthedocs.io/en/latest/wrappers.html#object-proxy

I actually think we *should* do that, and occasionally bug Graham Dumpleton 
about it, but while he's not opposed to the idea, it's also something that 
would take quite a bit of work (since odd edge cases that are tolerable in an 
opt-in third party module would probably need to be addressed for a standard 
library version).

For test cases like AsyncExitStack though, we instead just use custom type 
definitions, rather than the unittest.mock module.

Autospec'ed mocks are most attractive when we're dealing with object interfaces 
that are subject to a high rate of churn (since they make the tests more 
self-adjusting), and that isn't the case here: Python's syntactic support 
protocols rarely change, and when they do, preserving backwards compatibility 
with existing classes is typically a key requirement.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Yury Selivanov]

Yury Selivanov added the comment:

>> but at the same time rejected by the 'async with' statement.

> Perhaps unittest.mock (or type) needs to be adjusted to allow mocking via 
> spec= without subclassing?

Maybe. You should try to find discussions around this topic on python mailing 
lists and this tracker. If you find nothing then feel free to open an issue and 
add Michael Foord to nosy list.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Ilya Kulakov]

Ilya Kulakov added the comment:

> but at the same time rejected by the 'async with' statement.

Perhaps unittest.mock (or type) needs to be adjusted to allow mocking via spec= 
without subclassing?

> By all means you can submit a PR!

I'll take a look then.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Yury Selivanov]

Yury Selivanov added the comment:

> I'm not sure about type() to get a class object and calling __aenter__, 
> __aexit__ through it: that makes it hard to mock these classes as Mock's 
> spec= relies on __class__ and type() seem to ignore it (learned it a hard way.

Looking up __dunder__ methods on the class is how it should be done as that's 
how the rest of Python works.  And that's how ExitStack is currently 
implemented for synchronous "with" blocks.  We won't be able to change this 
behaviour even if we wanted to, so it stays.

Here's an example:

class Foo:
def __init__(self):
self.__aenter__ = ...
self.__aexit__ = ...

If we implement AsyncExitStack to lookup __anter__ directly on the object, then 
the above Foo class would be supported by it, but at the same time rejected by 
the 'async with' statement.

> Yury, I could take a second look and try to change this into a patch if 
> that's OK.

By all means you can submit a PR!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Ilya Kulakov]

Ilya Kulakov added the comment:

I'm not sure about type() to get a class object and calling __aenter__, 
__aexit__ through it: that makes it hard to mock these classes as Mock's spec= 
relies on __class__ and type() seem to ignore it (learned it a hard way.

Yury, I could take a second look and try to change this into a patch if that's 
OK.

--
nosy: +Ilya.Kulakov

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Yury Selivanov]

Yury Selivanov added the comment:

Overall, the approach looks fine. Alexander, do you want to make a PR to start 
working on adding this to 3.7?

--
versions:  -Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Alexander Mohr]

Alexander Mohr added the comment:

ok I've updated the gist with a base class and sync + async sub-classes.  The 
way it worked out I think is nice because we can have the same method names 
across both sync+async.  Let me know what you guys think!  btw, it seems the 
test_dont_reraise_RuntimeError test hangs even with the release version.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[STINNER Victor]

Changes by STINNER Victor :


--
nosy:  -haypo

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Guido van Rossum]

Changes by Guido van Rossum :


--
nosy:  -gvanrossum

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Yury Selivanov]

Yury Selivanov added the comment:

Looking at the code: we don't need to use iscoroutinefunction at all.  If we 
had separate methods for sync and async context managers, you can store the 
flag if a function is async or sync along with it.

So _exit_callbacks should store tuples `(is_async, cb)`, instead of just `cb`.

asyncio.iscoroutinefunction differs from inspect.iscoroutinefunction a little 
bit to support asyncio-specific debug coroutine wrapper functions.  We should 
avoid using any version of iscoroutinefunction here.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Yury Selivanov]

Yury Selivanov added the comment:

Alexander, 

You don't need asyncio.isocoroutinefunction. Please use 
inspect.iscoroutinefunction and inspect.iscoroutine.

Nick,

I'm +1 to have a separate class AsyncExitStack.

> - assume everything passed to it is a coroutine or an async context manager 
> by default

I would add a separate set of methods prefixed with 'a' to handle async context 
managers.

> - always require close() to be called via await

I'd only have one coroutine 'aclose()' that would internally close sync and 
async context managers in the right order.

> - either add synchronous variants of the default-to-async methods 
> (`enter_context_sync`, `push_sync`, `callback_sync`), or else make them 
> auto-adapt to handle both synchronous and asynchronous inputs

I'd use the 'a' prefix. We use it already to name magic methods: __aenter__, 
__aexit__, __aiter__.

> - rather than using `iscoroutinefunction`, instead always invoke callbacks 
> with await, and wrap synchronous callbacks supplied using the above methods 
> in simple one-shot iterators

I'd favour a more explicit approach: separate methods for handling sync and 
async.

Also, speaking about asyncio dependancy -- we shouldn't have it. Using 
asyncio.iscoroutinefunction is unnecessary, inspect.iscoroutinefunction should 
be used instead.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Alexander Mohr]

Alexander Mohr added the comment:

Thanks for the feedback Nick! If I get a chance I'll see about refactoring my 
gist into a base class and two sub-classes with the async supporting non-async 
but not vice-versa.  I think it will be cleaner.  Sorry I didn't spend too much 
effort on the existing gist as I tried quickly layering on async support to 
move on to my primary task.  After doing that I noticed that the code could use 
some refactoring, but only after taking the time deconstructing the impl to 
understand how the code works.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Nick Coghlan]

Nick Coghlan added the comment:

I quite like the idea of enhancing the existing ExitStack to also handle 
asynchronous operations rather than having completely independent 
implementations. However, a separate object may end up being simpler in 
practice as it allows developers to more explicitly declare their intent of 
writing async-friendly code.

Sketching out what a combined implementation based on the current ExitStack 
might look like:

- add suitable `__aenter__` and `__aexit__` implementations
- add `enter_context_async` (async def, called with await)
- add `push_aexit` (normal def, but given function is called with await)
- add `coroutine_callback` (normal def, but given coroutine is called with 
await)
- add `close_async` (async def, called with await)
- add a new internal state flag `_allow_async`. This would default to True, but 
be set to False when the synchronous `__enter__` is called. When it's false, 
attempting to register an async callback would raise RuntimeError. `__enter__` 
would also need to check any previously register contexts and callbacks, and 
complain if any of them require the use of `Await`
- keep track of which callbacks should be invoked as synchronous calls and 
which should be called via await
- update `close` to raise RuntimeError if it's asked to clean up asynchronous 
resources

That's really quite messy and hard to explain, and makes async code look quite 
different from the corresponding synchronous code.

The repeated checks of `self._allow_async` and `iscoroutinefunction` would also 
slow down existing synchronous code for no specific end user benefit.

By contrast, a dedicated AsyncExitStack object can:

- assume everything passed to it is a coroutine or an async context manager by 
default
- always require close() to be called via await
- either add synchronous variants of the default-to-async methods 
(`enter_context_sync`, `push_sync`, `callback_sync`), or else make them 
auto-adapt to handle both synchronous and asynchronous inputs
- rather than using `iscoroutinefunction`, instead always invoke callbacks with 
await, and wrap synchronous callbacks supplied using the above methods in 
simple one-shot iterators

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Nick Coghlan]

Changes by Nick Coghlan :


--
nosy: +giampaolo.rodola, gvanrossum, haypo, ncoghlan, yselivanov

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Vedran Čačić]

Vedran Čačić added the comment:

An example from "real life": I recently needed this when implementing a demo of 
dining philosophers using asyncio (when the order of locks should depend on 
comparison of fork ids, not be static). Of course, it wasn't hard to implement 
it directly, but still, it would be nice if I had it in stdlib.

Also, yes, it would probably be nicer if there was only one ExitStack with 
push_async and similar methods.

--
nosy: +veky

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Alexander Mohr]

Alexander Mohr added the comment:

created gist: https://gist.github.com/thehesiod/b8442ed50e27a23524435a22f10c04a0

I've now updated the imple to support both __aenter__/__aexit__ and 
__enter__/__exit__ so I don't need two ExitStacks

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Alexander Mohr]

Changes by Alexander Mohr :


Removed file: http://bugs.python.org/file46322/exit_stack.py

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue29302] add contextlib.AsyncExitStack

[Alexander Mohr]

New submission from Alexander Mohr:

ExitStack is a really useful class and would be a great to have an async 
version.  I've gone ahead and created an implementation based on the existing 
Python 3.5.2 implementation.  Let me know what you guys think. I think it would 
be possible to combine most of the two classes together if you guys think it 
would be useful.  Let me know if I can/should create a github PR and where to 
do that.

--
components: Library (Lib)
files: exit_stack.py
messages: 285687
nosy: thehesiod
priority: normal
severity: normal
status: open
title: add contextlib.AsyncExitStack
type: enhancement
versions: Python 3.6, Python 3.7
Added file: http://bugs.python.org/file46322/exit_stack.py

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com