asyncio.timeout() is not portable
It has come to my attention (https://github.com/KeepSafe/aiohttp/issues/877) that Python 3.5.2 (!) introduced a new context manager asyncio.timeout, which attaches a timeout to the current asyncio.Task. Any library or application that uses this feature will not be portable to other coroutine runners. with asyncio.timeout(1): result = await aiohttp.get('http://www.example.com') It's difficult to make this interface portable. We'd need to introduce a global thread-specific object that all of the coroutine runners could register the current task on, and define an interface that asyncio.timeout would call on that object to set the timeout. We could get rid of the global if the context manager used `async with` instead of `with`, since that would give us a way to communicate with the coroutine runner directly, but the net result is still that each runner needs to implement hooks for this feature (the hooks may or may not be generalizable to other features in the future). If our goal is portability across coroutine runners, then asyncio.timeout() needs to go back to the drawing board. If the goal is to extend asyncio.Task to be the one true coroutine runner then we could leave it as-is. For comparison, Tornado's equivalent to asyncio.timeout is tornado.gen.with_timeout(), and it has no special interactions with the coroutine runner: result = await with_timeout(timedelta(seconds=1), convert_yielded(aiohttp.get("http://www.example.com"))) (The convert_yielded call won't be needed in the next release of Tornado. The use of timedelta is because Tornado prefers absolute deadlines instead of relative timeouts, so that's how plain numbers are interpreted). -Ben
On Jun 5, 2016 8:57 AM, "Ben Darnell" <ben@bendarnell.com> wrote:
It has come to my attention (
https://github.com/KeepSafe/aiohttp/issues/877) that Python 3.5.2 (!) introduced a new context manager asyncio.timeout, which attaches a timeout to the current asyncio.Task. Any library or application that uses this feature will not be portable to other coroutine runners.
with asyncio.timeout(1): result = await aiohttp.get('http://www.example.com')
It's difficult to make this interface portable. We'd need to introduce a
global thread-specific object that all of the coroutine runners could register the current task on, and define an interface that asyncio.timeout would call on that object to set the timeout. We could get rid of the global if the context manager used `async with` instead of `with`, since that would give us a way to communicate with the coroutine runner directly, but the net result is still that each runner needs to implement hooks for this feature (the hooks may or may not be generalizable to other features in the future). Right -- fwiw curio's version of this uses either 'async with' (like you suggest) or acts as a wrapper (like tornado): http://curio.readthedocs.io/en/latest/reference.html#timeouts But this is consistent with these library's respective conventions: asyncio's style in general is to look up the event loop using global state that has to be kept synchronized, and here that's extended to looking up the current coroutine runner as well. Curio, OTOH, never manipulates global state. So to me this points to one of the fundamental differences between these approaches; it doesn't seem like something where one can just look at the timeout function in isolation. -n
So in the case of that Tornado example, who does the sleeping (or how is the exception scheduled)? We may not be able to retract this from 3.5.2 (RC1 goes out next weekend) but asyncio is still provisional until the 3.6 feature freeze in September. What's a better design? (FWIW asyncio.timeout() takes an optional loop parameter, but that's probably not helpful.) On Sun, Jun 5, 2016 at 8:56 AM, Ben Darnell <ben@bendarnell.com> wrote:
It has come to my attention (https://github.com/KeepSafe/aiohttp/issues/877) that Python 3.5.2 (!) introduced a new context manager asyncio.timeout, which attaches a timeout to the current asyncio.Task. Any library or application that uses this feature will not be portable to other coroutine runners.
with asyncio.timeout(1): result = await aiohttp.get('http://www.example.com')
It's difficult to make this interface portable. We'd need to introduce a global thread-specific object that all of the coroutine runners could register the current task on, and define an interface that asyncio.timeout would call on that object to set the timeout. We could get rid of the global if the context manager used `async with` instead of `with`, since that would give us a way to communicate with the coroutine runner directly, but the net result is still that each runner needs to implement hooks for this feature (the hooks may or may not be generalizable to other features in the future).
If our goal is portability across coroutine runners, then asyncio.timeout() needs to go back to the drawing board. If the goal is to extend asyncio.Task to be the one true coroutine runner then we could leave it as-is.
For comparison, Tornado's equivalent to asyncio.timeout is tornado.gen.with_timeout(), and it has no special interactions with the coroutine runner:
result = await with_timeout(timedelta(seconds=1), convert_yielded(aiohttp.get("http://www.example.com")))
(The convert_yielded call won't be needed in the next release of Tornado. The use of timedelta is because Tornado prefers absolute deadlines instead of relative timeouts, so that's how plain numbers are interpreted).
-Ben
_______________________________________________ Async-sig mailing list Async-sig@python.org https://mail.python.org/mailman/listinfo/async-sig Code of Conduct: https://www.python.org/psf/codeofconduct/
-- --Guido van Rossum (python.org/~guido)
On Sun, Jun 5, 2016 at 1:02 PM, Guido van Rossum <guido@python.org> wrote:
So in the case of that Tornado example, who does the sleeping (or how is the exception scheduled)?
The exception is scheduled with IOLoop.add_timeout (equivalent to asyncio's call_later). One difference between the two is that Tornado's with_timeout creates a new future (and a new coroutine runner) instead of cancelling the input future. The code is here: https://github.com/tornadoweb/tornado/blob/11bd951cc04747c0f03c71575d004f322...
We may not be able to retract this from 3.5.2 (RC1 goes out next weekend) but asyncio is still provisional until the 3.6 feature freeze in September. What's a better design?
I prefer a function that wraps an awaitable (as in Tornado's implementation), but this does have a performance impact. If we want to avoid that, then I think an `async with` context manager would be a better choice than an ordinary context manager, but I'd like to have a proof of concept implementation that works across asyncio and at least one other platform before committing to that design.
(FWIW asyncio.timeout() takes an optional loop parameter, but that's probably not helpful.)
No, it's not helpful because event loops and coroutine runners are independent. It's easy to find the current event loop (there are globals for that), but you can't really interact with the coroutine runner except via yield/await (unless we introduce new globals for this purpose) -Ben
On Sun, Jun 5, 2016 at 8:56 AM, Ben Darnell <ben@bendarnell.com> wrote:
It has come to my attention ( https://github.com/KeepSafe/aiohttp/issues/877) that Python 3.5.2 (!) introduced a new context manager asyncio.timeout, which attaches a timeout to the current asyncio.Task. Any library or application that uses this feature will not be portable to other coroutine runners.
with asyncio.timeout(1): result = await aiohttp.get('http://www.example.com')
It's difficult to make this interface portable. We'd need to introduce a global thread-specific object that all of the coroutine runners could register the current task on, and define an interface that asyncio.timeout would call on that object to set the timeout. We could get rid of the global if the context manager used `async with` instead of `with`, since that would give us a way to communicate with the coroutine runner directly, but the net result is still that each runner needs to implement hooks for this feature (the hooks may or may not be generalizable to other features in the future).
If our goal is portability across coroutine runners, then asyncio.timeout() needs to go back to the drawing board. If the goal is to extend asyncio.Task to be the one true coroutine runner then we could leave it as-is.
For comparison, Tornado's equivalent to asyncio.timeout is tornado.gen.with_timeout(), and it has no special interactions with the coroutine runner:
result = await with_timeout(timedelta(seconds=1), convert_yielded(aiohttp.get("http://www.example.com ")))
(The convert_yielded call won't be needed in the next release of Tornado. The use of timedelta is because Tornado prefers absolute deadlines instead of relative timeouts, so that's how plain numbers are interpreted).
-Ben
_______________________________________________ Async-sig mailing list Async-sig@python.org https://mail.python.org/mailman/listinfo/async-sig Code of Conduct: https://www.python.org/psf/codeofconduct/
-- --Guido van Rossum (python.org/~guido)
On Sun, Jun 5, 2016 at 10:24 AM, Ben Darnell <ben@bendarnell.com> wrote: [...]
No, it's not helpful because event loops and coroutine runners are independent. It's easy to find the current event loop (there are globals for that), but you can't really interact with the coroutine runner except via yield/await (unless we introduce new globals for this purpose)
Of course the reason it takes the loop argument is that in asyncio, event loops and coroutine runners aren't independent at all -- the loop is a global, and then the current coroutine runner is part of the loop state, so effectively the current coroutine runner is already a global: https://docs.python.org/3/library/asyncio-task.html#asyncio.Task.current_tas... In the curio approach they also aren't independent -- they don't have any magic globals, but for them the event loop and coroutine runner are the same object. And AFAICT the tornado.gen.with_timeout wrapper doesn't deal with coroutines/coroutine runners at all? I just read the source, but I couldn't fully follow it. If I write async def do_stuff(): try: await aiohttp.get("https://python.org") except: print("oops, cleaning up") raise else: print("did it!") await tornado.gen.with_timeout(timedelta(seconds=0.0001), do_stuff()) and the timeout fires, then is the output "oops, cleaning up", "did it!", or nothing at all? I guess the main case where coroutine runners are really independent from an event loop is when you have random little ad hoc coroutine runners that aren't directly integrated with an event loop at all. E.g., in my async_generator package the @async_generator decorator acts as a coroutine runner that adapts a regular coroutine into an async iterator, by intercepting special 'await yield_(...)' messages and transparently proxying other messages to the underlying coroutine runner. Before I read the source I was expecting that maybe tornado.gen.timeout_after would work in a broadly similar way, but apparently I was wrong :-). I don't have any argument here -- just trying to think aloud to understand the design space better... -n -- Nathaniel J. Smith -- https://vorpus.org
Regarding to asyncio event loop globalness. In my code I never use implicit global event loop but always pass the instance everywhere when needed. aiohttp as well as other aio-libs libraries supports optional loop=None parameter. The only reason why the loop is optional -- we are trying to follow asyncio conventions. But frankly speaking I found implicit event loop both non-obvious and error-prone. It would be nice to always pass event loop explicitly in asyncio-based code but ship has sailed years ago. On Sun, Jun 5, 2016 at 11:38 AM Nathaniel Smith <njs@pobox.com> wrote:
On Sun, Jun 5, 2016 at 10:24 AM, Ben Darnell <ben@bendarnell.com> wrote: [...]
No, it's not helpful because event loops and coroutine runners are independent. It's easy to find the current event loop (there are globals for that), but you can't really interact with the coroutine runner except via yield/await (unless we introduce new globals for this purpose)
Of course the reason it takes the loop argument is that in asyncio, event loops and coroutine runners aren't independent at all -- the loop is a global, and then the current coroutine runner is part of the loop state, so effectively the current coroutine runner is already a global:
https://docs.python.org/3/library/asyncio-task.html#asyncio.Task.current_tas...
In the curio approach they also aren't independent -- they don't have any magic globals, but for them the event loop and coroutine runner are the same object.
And AFAICT the tornado.gen.with_timeout wrapper doesn't deal with coroutines/coroutine runners at all? I just read the source, but I couldn't fully follow it. If I write async def do_stuff(): try: await aiohttp.get("https://python.org") except: print("oops, cleaning up") raise else: print("did it!") await tornado.gen.with_timeout(timedelta(seconds=0.0001), do_stuff()) and the timeout fires, then is the output "oops, cleaning up", "did it!", or nothing at all?
I guess the main case where coroutine runners are really independent from an event loop is when you have random little ad hoc coroutine runners that aren't directly integrated with an event loop at all. E.g., in my async_generator package the @async_generator decorator acts as a coroutine runner that adapts a regular coroutine into an async iterator, by intercepting special 'await yield_(...)' messages and transparently proxying other messages to the underlying coroutine runner. Before I read the source I was expecting that maybe tornado.gen.timeout_after would work in a broadly similar way, but apparently I was wrong :-).
I don't have any argument here -- just trying to think aloud to understand the design space better...
-n
-- Nathaniel J. Smith -- https://vorpus.org _______________________________________________ Async-sig mailing list Async-sig@python.org https://mail.python.org/mailman/listinfo/async-sig Code of Conduct: https://www.python.org/psf/codeofconduct/
-- Thanks, Andrew Svetlov
On Sun, Jun 5, 2016 at 11:38 AM, Nathaniel Smith <njs@pobox.com> wrote:
And AFAICT the tornado.gen.with_timeout wrapper doesn't deal with coroutines/coroutine runners at all?
It doesn't currently, but this is a bug. It should be creating a coroutine runner for the input task if it is not already a Future.
I just read the source, but I couldn't fully follow it. If I write async def do_stuff(): try: await aiohttp.get("https://python.org") except: print("oops, cleaning up") raise else: print("did it!") await tornado.gen.with_timeout(timedelta(seconds=0.0001), do_stuff()) and the timeout fires, then is the output "oops, cleaning up", "did it!", or nothing at all?
This would not produce any output: Tornado doesn't do asyncio-style cancellation (I've had enough bad experiences with KeyboardInterrupt cropping up at unexpected times). The inner task has its own coroutine runner and will continue after the outer task has stopped waiting for it. This allows the Future from the inner task to be reused (either in the same outer task or in another task), but it does require that some lower-level timeout mechanism also be present to prevent timed-out tasks from accumulating forever. This is not ideal and it would be good to allow the inner task to find out its been canceled (the reusability of Futures only matters in rare cases). -Ben
On Jun 5, 2016, at 11:56 AM, Ben Darnell <ben@bendarnell.com> wrote:
It has come to my attention (https://github.com/KeepSafe/aiohttp/issues/877) that Python 3.5.2 (!) introduced a new context manager asyncio.timeout, which attaches a timeout to the current asyncio.Task. Any library or application that uses this feature will not be portable to other coroutine runners.
with asyncio.timeout(1): result = await aiohttp.get('http://www.example.com')
It's difficult to make this interface portable. We'd need to introduce a global thread-specific object that all of the coroutine runners could register the current task on, and define an interface that asyncio.timeout would call on that object to set the timeout. We could get rid of the global if the context manager used `async with` instead of `with`, since that would give us a way to communicate with the coroutine runner directly, but the net result is still that each runner needs to implement hooks for this feature (the hooks may or may not be generalizable to other features in the future).
We’ve discussed this and decided that we should revert asyncio.Timeout from 3.5.2. I’ll do that later today. Yury
participants (5)
-
Andrew Svetlov -
Ben Darnell -
Guido van Rossum -
Nathaniel Smith -
Yury Selivanov