Skip to content

bpo-33649: First asyncio docs improvement pass #9142

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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 11, 2018
Merged

Conversation

1st1
Copy link
Member

@1st1 1st1 commented Sep 10, 2018

This is a first PR in the series

Rewritten/updated sections:

  • Event Loop APIs
  • Transports & Protocols
  • Streams
  • Exceptions
  • Policies
  • Queues
  • Subprocesses
  • Platforms

https://bugs.python.org/issue33649

Rewritten/updated sections:

* Event Loop APIs
* Transports & Protocols
* Streams
* Exceptions
* Policies
* Queues
* Subprocesses
* Platforms
@1st1 1st1 self-assigned this Sep 10, 2018
@1st1 1st1 requested review from ambv, asvetlov and willingc September 10, 2018 20:33
@bedevere-bot bedevere-bot added docs Documentation in the Doc dir awaiting merge labels Sep 10, 2018
Copy link
Contributor

@willingc willingc left a comment

Choose a reason for hiding this comment

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

@1st1 This looks like a great start. Instead of doing the polishing of wording as the review, let's either merge this and I can open another PR that polishes these files or I can push to your branch.

.. class:: WindowsProactorEventLoopPolicy

An alternative event loop policy that uses the
:class:`ProactorEventLoop` event loop implementation.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same issue re: link

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure I understand this comment :(

Copy link
Contributor

Choose a reason for hiding this comment

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

No worries. The link didn't seem to land somewhere useful. We can revisit this in another PR..

Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting, the link works on my docs build and points to the ProactorEventLoop docs...

and the event loop executes the next task.
An event loop runs in a thread (typically the main thread) and executes
all callbacks and tasks in its thread. While a task is running in the
event loop, no other task is running in the same thread. When a task
Copy link
Contributor

Choose a reason for hiding this comment

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

no other tasks may run in the...

An event loop runs in a thread (typically the main thread) and executes
all callbacks and tasks in its thread. While a task is running in the
event loop, no other task is running in the same thread. When a task
executes an ``await`` expression, the task gets suspended and the
Copy link
Contributor

Choose a reason for hiding this comment

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

the running task

@@ -122,7 +123,7 @@ To schedule a coroutine object from a different thread, the
future = asyncio.run_coroutine_threadsafe(coro_func(), loop)
result = future.result(timeout) # Wait for the result with a timeout

The :meth:`AbstractEventLoop.run_in_executor` method can be used with a thread pool
The :meth:`loop.run_in_executor` method can be used with a thread pool
Copy link
Contributor

Choose a reason for hiding this comment

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

ThreadPoolExecutor to execute a callback in a different thread so as not to block the event loop's main thread.

An event loop runs in a thread (typically the main thread) and executes
all callbacks and tasks in its thread. While a task is running in the
event loop, no other task is running in the same thread. When a task
executes an ``await`` expression, the task gets suspended and the
Copy link
Contributor

Choose a reason for hiding this comment

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

Technically the task can be suspended but if a future at the end of awaiting chain is ready no suspension is performed.

I described this behavior to many people. Stack Overflow also has questions like this.
Looks like we need to mention it here to avoid confusion.

@@ -182,7 +183,7 @@ Detect coroutine objects never scheduled
----------------------------------------

When a coroutine function is called and its result is not passed to
:func:`ensure_future` or to the :meth:`AbstractEventLoop.create_task` method,
:func:`ensure_future` or to the :meth:`loop.create_task` method,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we focus on asyncio.create_task() helper here?

@@ -204,7 +205,7 @@ Output in debug mode::
test()

The fix is to call the :func:`ensure_future` function or the
:meth:`AbstractEventLoop.create_task` method with the coroutine object.
:meth:`loop.create_task` method with the coroutine object.
Copy link
Contributor

Choose a reason for hiding this comment

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

asyncio.create_task() again?

@@ -279,7 +280,7 @@ coroutine in another coroutine and use classic try/except::
loop.run_forever()
loop.close()

Another option is to use the :meth:`AbstractEventLoop.run_until_complete`
Another option is to use the :meth:`loop.run_until_complete`
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we promote asyncio.run() here?

@1st1 1st1 added the skip news label Sep 11, 2018
@1st1
Copy link
Member Author

1st1 commented Sep 11, 2018

@willingc @asvetlov I haven't actually reviewed/updated the asyncio-dev.rst file yet, so I'll ignore the related review comments for now. Maybe we'll just remove the file, or remove whole sections from it, I just haven't looked closely at it yet.

Copy link
Contributor

@willingc willingc left a comment

Choose a reason for hiding this comment

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

@1st1 let's merge as is and we can do another PR for asyncio-dev.rst later this week.

@1st1 1st1 merged commit 7c7605f into python:master Sep 11, 2018
@1st1 1st1 deleted the docsimp branch September 11, 2018 16:54
@1st1
Copy link
Member Author

1st1 commented Sep 11, 2018

Merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants