bpo-38260: Add Docs on asyncio.run by eamanu · Pull Request #16337 · python/cpython

eamanu · 2019-09-23T21:05:20Z

Add docs about return and raise exception on asyncio.run

https://bugs.python.org/issue38260

Automerge-Triggered-By: @asvetlov

Add docs about return and raise exception on asyncio.run

asvetlov · 2019-09-23T22:17:19Z

Doc/library/asyncio-task.rst

    the end.  It should be used as a main entry point for asyncio
    programs, and should ideally only be called once.

+    Return the `base_events.run_until_complete()` that this return the


I think we don't need to mention a future and run_until_complete() here.
asyncio.run() returns a result of coro execution, that's it.

Hi @asvetlov Thanks for the review. I make the changes

corona10

I left some comments for you.
Please take a look

corona10 · 2019-09-24T16:37:16Z

Doc/library/asyncio-task.rst

    the end.  It should be used as a main entry point for asyncio
    programs, and should ideally only be called once.

+    Return a result of *coro* execution, or raise a RuntimeError if


RuntimeError should be

:exc:`RuntimeError`

Thanks I change it

corona10 · 2019-09-24T16:37:54Z

Doc/library/asyncio-task.rst


+    Return a result of *coro* execution, or raise a RuntimeError if
+    ``asyncio.run()`` is called from a running event loop, or a
+    ValueError if *coro* is not a courutine.


This also should be

:exc:`ValueError`

Change it too. thanks!

asvetlov

Thanks!

miss-islington · 2019-09-25T08:53:48Z

@eamanu: Status check is done, and it's a success ✅ .

miss-islington · 2019-09-25T08:53:52Z

Thanks @eamanu for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖

bedevere-bot · 2019-09-25T08:54:13Z

GH-16379 is a backport of this pull request to the 3.8 branch.

bedevere-bot · 2019-09-25T08:54:19Z

GH-16380 is a backport of this pull request to the 3.7 branch.

@asvetlov

Add docs about return and raise exception on asyncio.run https://bugs.python.org/issue38260 Automerge-Triggered-By: @asvetlov (cherry picked from commit 17deb16) Co-authored-by: Emmanuel Arias <[email protected]>

@asvetlov

Add docs about return and raise exception on asyncio.run https://bugs.python.org/issue38260 Automerge-Triggered-By: @asvetlov (cherry picked from commit 17deb16) Co-authored-by: Emmanuel Arias <[email protected]>

@asvetlov

Add docs about return and raise exception on asyncio.run https://bugs.python.org/issue38260 Automerge-Triggered-By: @asvetlov (cherry picked from commit 17deb16) Co-authored-by: Emmanuel Arias <[email protected]>

bedevere-bot · 2019-09-25T10:07:53Z

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 Debian PGO 3.8 has failed when building commit 4633355.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/220/builds/371) and take a look at the build logs.
Check if the failure is related to this commit (4633355) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/220/builds/371

Failed tests:

test_venv

Failed subtests:

test_with_pip - test.test_venv.EnsurePipTest

Summary of the results of the build (if available):

== Tests result: FAILURE then FAILURE ==

408 tests OK.

10 slowest tests:

test_concurrent_futures: 2 min 37 sec
test_multiprocessing_spawn: 1 min 51 sec
test_multiprocessing_forkserver: 1 min 14 sec
test_multiprocessing_fork: 1 min 6 sec
test_asyncio: 49 sec 744 ms
test_tokenize: 44 sec 417 ms
test_io: 36 sec 477 ms
test_subprocess: 34 sec 233 ms
test_lib2to3: 28 sec 560 ms
test_nntplib: 25 sec 21 ms

1 test failed:
test_venv

14 tests skipped:
test_devpoll test_gdb test_ioctl test_kqueue test_msilib
test_ossaudiodev test_startfile test_tix test_tk test_ttk_guionly
test_winconsoleio test_winreg test_winsound test_zipfile64

1 re-run test:
test_venv

Total duration: 10 min 15 sec

Click to see traceback logs

Traceback (most recent call last):
  File "/var/lib/buildbot/slaves/enable-optimizations-bot/3.8.gps-debian-profile-opt.pgo/build/Lib/test/test_venv.py", line 490, in test_with_pip
    self.do_test_with_pip(False)
  File "/var/lib/buildbot/slaves/enable-optimizations-bot/3.8.gps-debian-profile-opt.pgo/build/Lib/test/test_venv.py", line 438, in do_test_with_pip
    self.fail(msg.format(exc, details))
AssertionError: Command '['/tmp/tmpthuhiyk0/bin/python', '-Im', 'ensurepip', '--upgrade', '--default-pip']' returned non-zero exit status 1.


Traceback (most recent call last):
  File "/var/lib/buildbot/slaves/enable-optimizations-bot/3.8.gps-debian-profile-opt.pgo/build/Lib/test/test_venv.py", line 430, in do_test_with_pip
    self.run_with_capture(venv.create, self.env_dir,
  File "/var/lib/buildbot/slaves/enable-optimizations-bot/3.8.gps-debian-profile-opt.pgo/build/Lib/test/test_venv.py", line 76, in run_with_capture
    func(*args, **kwargs)
subprocess.CalledProcessError: Command '['/tmp/tmpthuhiyk0/bin/python', '-Im', 'ensurepip', '--upgrade', '--default-pip']' returned non-zero exit status 1.


Traceback (most recent call last):
  File "/var/lib/buildbot/slaves/enable-optimizations-bot/3.8.gps-debian-profile-opt.pgo/build/Lib/test/test_venv.py", line 430, in do_test_with_pip
    self.run_with_capture(venv.create, self.env_dir,
  File "/var/lib/buildbot/slaves/enable-optimizations-bot/3.8.gps-debian-profile-opt.pgo/build/Lib/test/test_venv.py", line 76, in run_with_capture
    func(*args, **kwargs)
subprocess.CalledProcessError: Command '['/tmp/tmpkxolwwyz/bin/python', '-Im', 'ensurepip', '--upgrade', '--default-pip']' returned non-zero exit status 1.


Traceback (most recent call last):
  File "/var/lib/buildbot/slaves/enable-optimizations-bot/3.8.gps-debian-profile-opt.pgo/build/Lib/test/test_venv.py", line 490, in test_with_pip
    self.do_test_with_pip(False)
  File "/var/lib/buildbot/slaves/enable-optimizations-bot/3.8.gps-debian-profile-opt.pgo/build/Lib/test/test_venv.py", line 438, in do_test_with_pip
    self.fail(msg.format(exc, details))
AssertionError: Command '['/tmp/tmpkxolwwyz/bin/python', '-Im', 'ensurepip', '--upgrade', '--default-pip']' returned non-zero exit status 1.

aeros · 2019-09-25T21:36:02Z

Doc/library/asyncio-task.rst


+    Return a result of *coro* execution, or raise a :exc:`RuntimeError`
+    if ``asyncio.run()`` is called from a running event loop, or a
+    :exc:`ValueError` if *coro* is not a courutine.


Thanks for the adding the additional information @eamanu, but I noticed a couple of issues with the section added.

The more apparent one was a spelling typo, "courutine" should be "coroutine" (misspelled here and in the docstring). Also, it's helpful to link to the glossary using:

:term:`coroutine`

Also, while this section is helpful (particularly with specifying the RuntimeError and ValueError scenarios), I think it could be phrased significantly better. I would suggest the following changes:

Within the new event loop, *coro* is executed, returning the result. If ``asyncio.run()`` is called within a running event loop, a :exc:`RuntimeError` is raised. If *coro* is not a coroutine, a :exc:`ValueError` is raised.

Personally, I think it's especially important to phrase this section of the documentation as well as possible, since it's likely one of the most read sections for the entire asyncio module.

Let me know what you think @asvetlov and @1st1. If you approve of the suggestions, I'll open a new PR that makes the changes I mentioned above in the docs for ascynio.run() and in the docstring as well (since this one was already merged and backported).

Grammar tip: When chaining together multiple instances of "or", each of the items should be separated by a comma, but only the last one is supposed to have the "or". For example: "item1, item2, or item3" instead of "item1, or item2, or item3". The section added to the documentation in this PR used the latter.

I'm not sure I follow "Within the new event loop, coro is executed, returning the result." -- I can't parse it. I'd keep the original one "Return the result of coro execution."

IMO it's not necessary to document RuntimeError and ValueError; "This function cannot be called when another asyncio event loop is running in the same thread." already covers that.

@1st1 Okay, I'll make the suggested changes in #16403.

Edit: Actually, I think specifying that ValueError is raised when something other than a coroutine is passed might be useful (for basic debugging purposes). Do you think that can remain since it's not covered by the existing section?

aeros · 2019-09-25T21:41:56Z

Also in case it wasn't obvious, the test_venv failure is completely unrelated to the PR. This failure has been happening intermittently for the buildbots recently. I believe a similar failure with test_venv recently happened in a PR from @corona10 and he created a bpo issue for it.

asvetlov · 2019-09-25T21:45:55Z

PR with improvement is welcome! @aeros167 your proposals make sense

aeros · 2019-09-25T21:47:50Z

Sounds good! I'll open one soon.

Edit: The changes are applied in #16403.

@asvetlov

Add docs about return and raise exception on asyncio.run https://bugs.python.org/issue38260 Automerge-Triggered-By: @asvetlov

Add Docs on asyncio.run

39cd32b

Add docs about return and raise exception on asyncio.run

eamanu requested review from 1st1 and asvetlov as code owners September 23, 2019 21:05

the-knights-who-say-ni added the CLA signed label Sep 23, 2019

bedevere-bot added the awaiting review label Sep 23, 2019

asvetlov reviewed Sep 23, 2019

View reviewed changes

Fix documentation according to @asvetlov

1ba3164

brandtbucher added needs backport to 3.7 docs Documentation in the Doc dir labels Sep 23, 2019

corona10 requested changes Sep 24, 2019

View reviewed changes

bedevere-bot added awaiting core review and removed awaiting review labels Sep 24, 2019

Add @corona10 comments

9dc8cb3

asvetlov approved these changes Sep 25, 2019

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting core review labels Sep 25, 2019

asvetlov added 🤖 automerge skip news labels Sep 25, 2019

miss-islington merged commit 17deb16 into python:master Sep 25, 2019

bedevere-bot removed the awaiting merge label Sep 25, 2019

bedevere-bot removed the needs backport to 3.8 label Sep 25, 2019

bedevere-bot removed the needs backport to 3.7 label Sep 25, 2019

aeros reviewed Sep 25, 2019

View reviewed changes

This was referenced Sep 25, 2019

Fix and improve asyncio.run() docs #16403

Merged

bpo-38267: Add thread timeout parameter to loop.shutdown_default_executor() #16360

Closed

jacobneiltaylor pushed a commit to jacobneiltaylor/cpython that referenced this pull request Dec 5, 2019

bpo-38260: Add Docs on asyncio.run (pythonGH-16337)

396bd14

Add docs about return and raise exception on asyncio.run https://bugs.python.org/issue38260 Automerge-Triggered-By: @asvetlov

Uh oh!

Conversation

eamanu commented Sep 23, 2019 • edited by miss-islington Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asvetlov Sep 23, 2019

Choose a reason for hiding this comment

Uh oh!

eamanu Sep 23, 2019

Choose a reason for hiding this comment

Uh oh!

corona10 left a comment

Choose a reason for hiding this comment

Uh oh!

corona10 Sep 24, 2019

Choose a reason for hiding this comment

Uh oh!

eamanu Sep 25, 2019

Choose a reason for hiding this comment

Uh oh!

corona10 Sep 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eamanu Sep 25, 2019

Choose a reason for hiding this comment

Uh oh!

asvetlov left a comment

Choose a reason for hiding this comment

Uh oh!

miss-islington commented Sep 25, 2019

Uh oh!

miss-islington commented Sep 25, 2019

Uh oh!

bedevere-bot commented Sep 25, 2019

Uh oh!

bedevere-bot commented Sep 25, 2019

Uh oh!

bedevere-bot commented Sep 25, 2019

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Uh oh!

aeros Sep 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

1st1 Sep 27, 2019

Choose a reason for hiding this comment

Uh oh!

aeros Sep 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aeros commented Sep 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asvetlov commented Sep 25, 2019

Uh oh!

aeros commented Sep 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

eamanu commented Sep 23, 2019 •

edited by miss-islington

Loading

corona10 Sep 24, 2019 •

edited

Loading

aeros Sep 25, 2019 •

edited

Loading

aeros Sep 27, 2019 •

edited

Loading

aeros commented Sep 25, 2019 •

edited

Loading

aeros commented Sep 25, 2019 •

edited

Loading