GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — bpo-34831: Asyncio tutorial by cjrh · Pull Request #9748 · python/cpython Conversation
|
|
cc @ambv -- Łukasz, would be great if you could take a look at this too and share some of your thoughts/experience. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
also @elprans please take a look. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
a7a9f25 to
4b47460
Compare
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Thanks @cjrh. A few comments so far.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
9f21e81 to
30d158d
Compare
|
Docs failure could be due to recent logging related changes to fix Sphinx deprecation warnings in ee171a2 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Upstream issue : https://bugs.python.org/issue35036 Running (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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
a6668dd to
7c95086
Compare
There was an error while loading. Please reload this page.
7c95086 to
8c2f3ff
Compare
|
Hi @cjrh, Thanks for updating the PR. I will do my best to review it this week. Hope all is well in your world. ☀️ |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Yeah, it's a huge improvement; I'll also try to find time to review this before 3.8 final. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thanks for the update, I'll rebase that soon. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
576ef44 to
26cc634
Compare
|
This is huge, thank you, @cjrh! I'm a bit busy at the sprints right now, I'll review this soon. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| Async Functions, And Other Syntax Features | ||
| ========================================== | ||
|
|
||
| Regular Python functions are created with the keyword ``def``, |
There was a problem 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`, |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| 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 |
There was a problem 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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
|
||
| 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 |
There was a problem 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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| 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. |
There was a problem 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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| ------------------------- | ||
|
|
||
| The ``run`` function is only good for executing an async function | ||
| from "synchronous" code; and this is usually only used to execute |
There was a problem 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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| Using A Queue To Move Data Between Long-Lived Tasks | ||
| --------------------------------------------------- | ||
|
|
||
| TODO |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
TODO?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| Best Practices For Timeouts | ||
| --------------------------- | ||
|
|
||
| - start with ``asyncio.wait_for()`` |
There was a problem 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()` |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| --------------------------- | ||
|
|
||
| - start with ``asyncio.wait_for()`` | ||
| - also look at ``asyncio.wait()``, and what to do if not all tasks |
There was a problem 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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| -------------------------- | ||
|
|
||
| - app shutdown | ||
| - how to handle CancelledError and then close sockets |
There was a problem 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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| ... | ||
|
|
||
| async def main(): | ||
| server = await asyncio.start_server( |
There was a problem 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()
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I suggest splitting this combo PR into smaller sections. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I am closing this as this will be split into smaller PRs as discussed in the issue. Thanks for the PR! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.