FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-32314: Implement asyncio.run() by 1st1 · Pull Request #4852 · python/cpython · GitHub

/ cpython Public

bpo-32314: Implement asyncio.run() - #4852

Merged
1st1 merged 2 commits into
python:masterfrom
1st1:aio_run
Dec 14, 2017
Merged

bpo-32314: Implement asyncio.run()#4852
1st1 merged 2 commits into
python:masterfrom
1st1:aio_run

Conversation

1st1 commented Dec 13, 2017
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

Comment thread Lib/asyncio/runners.py

loop = events.new_event_loop()
try:
events.set_event_loop(loop)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

set_event_loop() cannot fail (it has a couple asserts but we can neglect this fact).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'll merge as-is then.

Comment thread Lib/asyncio/runners.py
loop = events.new_event_loop()
try:
events.set_event_loop(loop)
loop.set_debug(debug)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Never fails

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

Comment thread Doc/library/asyncio-task.rst Outdated
@@ -128,17 +143,15 @@ using the :meth:`sleep` function::
import datetime

async def display_date(loop):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

the loop argument should be removed now

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Good catch!

print(datetime.datetime.now())
if (loop.time() + 1.0) >= end_time:
break
await asyncio.sleep(1)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

this is a bit more concise

         while (loop.time() + 1.0) < end_time:
              print(datetime.datetime.now())
              await asyncio.sleep(1)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

ilevkivskyi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks! It will be great to have this in 3.7.

1st1 commented Dec 13, 2017

Copy link
Copy Markdown
Member Author

@asvetlov Andrew, I'll wait for your comments.

1st1 commented Dec 13, 2017

Copy link
Copy Markdown
Member Author

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

1st1 merged commit 02a0a19 into python:master Dec 14, 2017
1st1 deleted the aio_run branch December 14, 2017 14:42
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants


Back | FazBrowse Home | New Git URL