bpo-32314: Implement asyncio.run() by 1st1 · Pull Request #4852 · python/cpython (original) (raw)
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation16 Commits2 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
| loop = events.new_event_loop() |
| try: |
| events.set_event_loop(loop) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set_event_loop() cannot fail (it has a couple asserts but we can neglect this fact).
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure it can. It calls policy.set_event_loop(), and given the fact that users can provide their own buggy policies, we can't really say that set_event_loop is 100% safe.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a flag of broken third party loop implementation, not user code problem.
Buggy loop can do weird things, I expect asyncio will be broken somehow anyway.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's your point?
The point of this code is to always close the loop, no matter what. What benefit is there in moving set_event_loop() one line up?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anyways, if you think that the code will read better if set_event_loop is outside of the try block I can move it. We call set_event_loop in the finally block along with the loop.close(), so as you say: if these things are broken then nothing will actually work anyways.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, it is very minor thing.
From my perspective wrapping loop.run_until_complete(main) only increases code readability a little but I can live with current implementation too.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll merge as-is then.
| loop = events.new_event_loop() |
|---|
| try: |
| events.set_event_loop(loop) |
| loop.set_debug(debug) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never fails
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, it can, if, say, I have a bug in uvloop.Loop.set_debug method. I prefer to assume nothing, when it's possible to set custom policies and custom event loops.
| @@ -128,17 +143,15 @@ using the :meth:`sleep` function:: |
|---|
| import datetime |
| async def display_date(loop): |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the loop argument should be removed now
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
| @@ -128,17 +143,15 @@ using the :meth:`sleep` function:: |
|---|
| import datetime |
| async def display_date(loop): |
| loop = asyncio.get_running_loop() |
| end_time = loop.time() + 5.0 |
| while True: |
| print(datetime.datetime.now()) |
| if (loop.time() + 1.0) >= end_time: |
| break |
| await asyncio.sleep(1) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a bit more concise
while (loop.time() + 1.0) < end_time:
print(datetime.datetime.now())
await asyncio.sleep(1)Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can replace this while loop with for _ in range(5) and the example will only become clearer.
We'll have a separate pass over asyncio docs before 3.7 is released. We'll try to come up with better examples and improve the current ones. So for now, I'd keep this snippet as is.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! It will be great to have this in 3.7.
@asvetlov Andrew, I'll wait for your comments.
@ilevkivskyi Thanks! If you have any ideas about any APIs that would be great to have in asyncio, please open an issue. I have some time to work on asyncio and we still have a timeframe to make things happen.
