-
-
Notifications
You must be signed in to change notification settings - Fork 178
Add asyncio.run() and asyncio.run_forever() functions. #465
base: master
Are you sure you want to change the base?
Conversation
If my voice counts, I am definitely +1 on having |
+1, definitely! About async def wait_for_interrupt():
loop = asyncio.get_event_loop()
future = loop.create_future()
loop.add_signal_handler(signal.SIGINT, future.set_result, None)
try:
await future
finally:
loop.remove_signal_handler(signal.SIGINT) |
+1, LGTM |
This would work to support signals. I want try:
await asyncio.forever()
finally:
# important cleanup code code TBH I don't want to distract ourselves with
For sure. As I explained in the first message, if Guido is in favour of the idea I'll add tests/docs/etc. |
Sure, add this. I don't have time for a review though.
|
I've experimented a little bit, and it turns out that it's not that hard to implement the With the latest commit it's possible to write coroutines like this: async def foo():
print('hi')
try:
await asyncio.forever()
except KeyboardInterrupt:
await asyncio.sleep(1)
print('bye')
asyncio.run(foo()) The change modifies |
So forever is just sleep infinity? Why do we need that?
…--Guido (mobile)
|
It's similar but not the same. It's designed to replace uses of It allows to safely transform this: loop = asyncio.get_event_loop()
server = loop.run_until_complete(
asyncio.start_server(handle_echo, '127.0.0.1', 8888, loop=loop))
try:
loop.run_forever()
except KeyboardInterrupt:
pass
# Close the server
server.close()
loop.run_until_complete(server.wait_closed())
loop.close() into this: async def main():
server = await asyncio.start_server(handle_echo, '127.0.0.1', 8888)
try:
await asyncio.forever()
except KeyboardInterrupt:
pass
server.close()
await server.wait_closed()
asyncio.run(main()) The former example that uses The key difference from 'sleep(inf) Having all program bootstrap logic defined in one coroutine is easier than using
If you don't see any pitfalls with |
-1. Something that's only meant to be used at the top level doesn't deserve
to be a coroutine IMO.
And even if we have forever() I think you should be able to get the same
behavior (relative to KeyboardInterrupt) with sleep(100000).
|
A few comments about
@1st1 I'm not sure it makes sense to have an asynchronous equivalent to
But if the loop is stopped, how can the coroutine keep running? In the current implementation, it is restored by running the loop a second time. So I would argue that a |
Normally yes, I agree. Although I don't think this argument fully applies to this particular use case. The idea is to add APIs to make it possible to move the application bootstrap logic (the "main" function") into a coroutine. The design of
I see your point. Unfortunately it's not possible to implement this behaviour in What if we rename async def main():
server = await asyncio.start_server(handle_echo, '127.0.0.1', 8888)
try:
await asyncio.interrupted()
finally:
server.close()
await server.wait_closed()
asyncio.run(main())
The function is supposed be used to launch your main program coroutine and we recommend to use asyncio in the main thread. And yes, subprocesses don't properly work when the loop isn't running in the main thread. I don't think that lifting this restriction would help anyone to be honest.
Well, we run
Can't do that.
I'm -1 on all three.
It's up to the documentation -- the idea is that Please study my example code in this comment and in #465 (comment). |
+1 for keeping |
That's also ambiguous (is it a verb or a noun?). And the issue of whether
it would just be a shorthand for sleep(inf) is still unsettled.
|
I think I came up with a solution, please see below.
We can't fix Anyways, I think we can modify With that we can have this: async def main():
server = await asyncio.start_server(handle_echo, '127.0.0.1', 8888)
try:
yield
finally:
server.close()
await server.wait_closed()
asyncio.run(main()) I think this is a great idea because:
The |
I think it should rather be |
I like this, disregard my previous comment. |
The latest patch is really great. |
IIRC people are currently using a bare yield as the equivalent of sleep(0) I really am very concerned that we're going to break things at the very |
Also frankly run_in_executor() is a pretty clumsy API due to the initial parameter that is usually None. Is it really important enough to have this? (Again, I worry that this is a panic response rather than something well thought out.) |
But they use it in old style generator-based coroutines: @coroutine
def foo():
yield # this is a NOP yield async def main():
try:
yield # <- this is a yield from an async gen, something new in 3.6 Asynchronous generators can do any kind of yields they want -- there is no backwards compatibility issue here. In fact, I'm thinking about making
I wouldn't say that it's Nathaniel's post that caused this PR. I've been unhappy about the loop for a long time. I think I first proposed to fix |
Yes, I've been thinking about |
Oh, it's async generators. Too subtle. We did fix get_event_loop(), and we're all happy with that. Can we just stop now please? |
I would really prefer option 2: two separate functions -- |
I've modified this PR to add just two functions:
This PR also adds unittests. FWIW I used a custom asyncio policy to control the loop that the new functions use during tests, and it worked great. |
|
||
asyncio.run(main()) | ||
""" | ||
if events._get_running_loop() is not None: |
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.
Maybe these two checks (no running loop and main thread) that appear here and in run_forever()
could be factored out to a helper function like you did for _cleanup(loop)
?
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.
I want to customize the error message for each function, so I guess a little bit of copy/paste is fine.
"asyncio.run_forever() cannot be called from a running event loop") | ||
if not isinstance(threading.current_thread(), threading._MainThread): | ||
raise RuntimeError( | ||
"asyncio.run() must be called from the main thread") |
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.
Should be also run_forever()
here, not run()
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.
Will fix.
except StopAsyncIteration as ex: | ||
return | ||
if ret is not None: | ||
raise RuntimeError("only empty yield is supported") |
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.
Maybe "asyncio.run_forever() supports only asynchronous generators with empty yield"?
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.
Agree, will fix.
yielded_twice = True | ||
|
||
if yielded_twice: | ||
raise RuntimeError("only one yield is supported") |
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.
Maybe "asyncio.run_forever() supports asynchronous generators with only one yield"?
Thank you Yury, it looks great! I left few small comments concerning error messages. Also I think maybe it is worth adding a note somewhere in docs for people who are already familiar with (This is obvious if one looks at the code, but otherwise it might be not clear why such design was chosen.) |
Sure, we'll update the docs! |
We have very long discussion here. |
3.6b4 is scheduled for tomorrow, if there is a chance this goes into 3.6, then it probably makes sense to merge this before that time. |
Can this PR be moved to python/cpython? |
IIUC, Yury is working on a PEP now that will cover the features in this PR. |
@ilevkivskyi on your example above on the exception I would actually name the special function differently. instead of : def serve(mgr):
# Set-up loop
loop.run_until_complete(mgr.__aenter__())
try:
loop.run_forever()
except:
result = loop.run_until_complete(mgr.__aexit__(*sys.exc_info()))
else:
result = loop.run_until_complete(mgr.__aexit__(None, None, None))
# Careful clean-up I would do: def serve(mgr):
# Set-up loop
loop.run_until_complete(mgr.__aenter__())
try:
loop.run_forever()
except:
result = loop.run_until_complete(mgr.__on_error__(*sys.exc_info()))
else:
result = loop.run_until_complete(mgr.__aexit__(None, None, None))
# Careful clean-up So that way if they do not have or define an Or another way is to have some sort of decorator that would register an function with asyncio that would handle the exception message so that way asyncio would not error again if they do not have |
This PR adds two new APIs:
asyncio.run()
andasyncio.run_in_executor()
. Ideally, if possible, I'd like to have them in 3.6. If I have a green light on the idea, I'll update the patch to add unittests.One of the main complaints that users have about asyncio is the situation around the event loop. We currently do a poor job explaining how exactly it should be used and how to structure asyncio programs in general.
With the recent update of
asyncio.get_event_loop()
we can now explain people that passing event loop explicitly is unnecessary, and that library APIs should be designed around coroutines.I think we need to add two more functions to make the loop disappear from most asyncio programs.
asyncio.run_in_executor()
coroutine: maps directly to the equivalentloop.run_in_executor()
. The idea is that people don't need an event loop to use the function:asyncio.run()
function: run a coroutine taking care of the asyncio event loop.Pros:
Simplification of the documentation: I'm working on an update, and one of the things that bothers me that to each and every example have to have a piece of code that manages the loop. For example:
The problem is that the above snippet isn't fully correct, we should have a
try-finally
block to ensure thatloop.close()
is always called. Even in the docs we don't do that. Withasyncio.run()
the snippet becomes much shorter:It's currently hard to experiment with asyncio in the REPL, because
get_event_loop
andrun_until_complete
are rather long names to type. Withasyncio.run()
:And
asyncio.run()
can be called multiple times.Asynchronous generators are properly cleaned-up.
loop.shutdown_asyncgens()
is a somewhat low-level advanced API, and I expect something that a lot of people will forget to use.The function promotes a coroutine-centric design. In many cases, is is possible to bootstrap an asyncio program with just one coroutine.
Cons:
It's not possible to completely substitute
loop.run_forever()
. One of the documented patterns in asyncio is to userun_forever
to bootstrap servers:To support cases like this, we'll need to add another API. One of the ideas that I have (not for 3.6!) is to add
asyncio.forever()
awaitable, so that the above example could be translated to:Adding
asyncio.forever()
would require us to add new APIs to event loop, and it is something that clearly requires a thorough review process (I'm thinking about writing a PEP).However, we can probably add
asyncio.run()
function in 3.6 to cover some use cases, and enhance it further in 3.7.@gvanrossum, what do you think?