bpo-34831: Asyncio tutorial by cjrh · Pull Request #9748 · 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
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 }})
cc @ambv -- Łukasz, would be great if you could take a look at this too and share some of your thoughts/experience.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @cjrh. A few comments so far.
Docs failure could be due to recent logging related changes to fix Sphinx deprecation warnings in ee171a2
Upstream issue : https://bugs.python.org/issue35036
PR : #10024
Running make suspicious with the fix locally for reference :
(venv) ➜ Doc git:(0e82bc95f2) ✗ make suspicious mkdir -p build Building NEWS from Misc/NEWS.d with blurb PATH=./venv/bin:$PATH sphinx-build -b suspicious -d build/doctrees -D latex_elements.papersize= . build/suspicious Running Sphinx v1.8.1 loading pickled environment... done loading ignore rules... done, 343 rules loaded building [mo]: targets for 0 po files that are out of date building [suspicious]: targets for 484 source files that are out of date updating environment: [] 0 added, 11 changed, 0 removed reading sources... [100%] whatsnew/changelog looking for now-outdated files... none found pickling environment... done checking consistency... done preparing documents... done writing output... [ 30%] library/asyncio-tutorial/case-study-chat-server /Users/karthikeyansingaravelan/stuff/python/cpython/Doc/venv/lib/python3.7/site-packages/sphinx/application.py:388: RemovedInSphinx20Warning: app.warning() is now deprecated. Use sphinx.util.logging instead. RemovedInSphinx20Warning) writing output... [100%] whatsnew/index WARNING: [library/asyncio-tutorial/case-study-chat-server:49] ":TODO" found in " (ref:TODO) to create a TCP server very" build finished with problems, 1 warning. make[1]: *** [build] Error 1 Suspicious check complete; look for any errors in the above output or in build/suspicious/suspicious.csv. If all issues are false positives, append that file to tools/susp-ignored.csv. make: *** [suspicious] Error 1
Hi @cjrh, Thanks for updating the PR. I will do my best to review it this week. Hope all is well in your world. ☀️
Yeah, it's a huge improvement; I'll also try to find time to review this before 3.8 final.
The unreadable build failure is due to Sphinx deprecating a function, I'm currently fixing it, but in the meantime here's the error you have to fix (or add to Doc/tools/susp-ignored.csv):
WARNING: [distutils/examples:267] "`" found in "This is the description of the ``foobar`` package."
WARNING: [library/asyncio-tutorial/running-async-functions:131] "`" found in " task1 = func1() // In Python: `task1 = create_task(func1())`"
WARNING: [library/asyncio-tutorial/running-async-functions:131] "`" found in " task2 = func2() // In Python: `task2 = create_task(func2())`"
WARNING: Found 1/355 unused rules:
distutils/examples,,`,This is the description of the ``foobar`` package.
@cjrh You can now rebase on top of master, since e1786b5 the suspicious.py script is fixed.
Thanks for the update, I'll rebase that soon.
This is huge, thank you, @cjrh!
I'm a bit busy at the sprints right now, I'll review this soon.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very impressive.
Please use a new stream API instead of legacy reader/writer.
| Async Functions, And Other Syntax Features |
|---|
| ========================================== |
| Regular Python functions are created with the keyword ``def``, |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Regular Python functions are created with the keyword ``def``, |
|---|
| Regular Python functions are created with the keyword :keyword:`def`, |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushing :keyword: everywhere for building a link doesn't make sense but the first occurrence in a page can be a link, it's convenient maybe.
| asyncio.run(f(1, 2)) |
|---|
| The first difference is that the function declaration is spelled |
| ``async def``. The second difference is that async functions cannot be |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ``async def``. The second difference is that async functions cannot be |
|---|
| :keyword:`async def`. The second difference is that async functions cannot be |
| The first difference is that the function declaration is spelled |
| ``async def``. The second difference is that async functions cannot be |
| executed by simply evaluating them. Instead, we use the ``run()`` function |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| executed by simply evaluating them. Instead, we use the ``run()`` function |
|---|
| executed by simply evaluating them. Instead, we use the :func:`~asyncio.run` function |
| The first difference is that the function declaration is spelled |
|---|
| ``async def``. The second difference is that async functions cannot be |
| executed by simply evaluating them. Instead, we use the ``run()`` function |
| from the ``asyncio`` module. |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| from the ``asyncio`` module. |
|---|
| from the :mod:`asyncio` module. |
| ------------------------- |
|---|
| The ``run`` function is only good for executing an async function |
| from "synchronous" code; and this is usually only used to execute |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| from "synchronous" code; and this is usually only used to execute |
|---|
| from *synchronous* code; and this is usually only used to execute |
| Using A Queue To Move Data Between Long-Lived Tasks |
|---|
| --------------------------------------------------- |
| TODO |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO?
| Best Practices For Timeouts |
|---|
| --------------------------- |
| - start with ``asyncio.wait_for()`` |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - start with ``asyncio.wait_for()`` |
|---|
| - start with :func:`asyncio.wait_for()` |
| --------------------------- |
|---|
| - start with ``asyncio.wait_for()`` |
| - also look at ``asyncio.wait()``, and what to do if not all tasks |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - also look at ``asyncio.wait()``, and what to do if not all tasks |
|---|
| - also look at :func:`asyncio.wait()`, and what to do if not all tasks |
| -------------------------- |
|---|
| - app shutdown |
| - how to handle CancelledError and then close sockets |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - how to handle CancelledError and then close sockets |
|---|
| - how to handle :exc:`~asyncio.CancelledError` and then close sockets |
| ... |
|---|
| async def main(): |
| server = await asyncio.start_server( |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New API:
async with asyncio.StreamServer(client_connected_cb, 'localhost', 9011) as server:
await server.serve_forever()
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.
Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.
I suggest splitting this combo PR into smaller sections.
For example, async_functions.rst is finished and almost ready for merging after a little polishing.
It makes a value in itself, we can process it fast.
I am closing this as this will be split into smaller PRs as discussed in the issue. Thanks for the PR!