Skip to content

Conversation

@aeros
Copy link
Contributor

@aeros aeros commented May 17, 2020

Implements asyncio.to_thread, a coroutine for asynchronously running IO-bound functions in a separate thread without blocking the event loop. See the discussion starting from here in GH-18410 for context.

https://bugs.python.org/issue32309

Automerge-Triggered-By: @aeros

Co-authored-by: Yury Selivanov <yury@edgedb.com>
@aeros
Copy link
Contributor Author

aeros commented May 17, 2020

/cc @1st1

@1st1
Copy link
Member

1st1 commented May 18, 2020

This looks great to me. Please don't merge before @asvetlov and @cjerdonek approve.

@1st1 1st1 requested a review from methane May 18, 2020 06:23
@tmewett
Copy link

tmewett commented May 18, 2020

Is the name finalised? I saw the discussion about run_in_thread being a bit ambiguous but personally I don't think to_thread is very clear either.

I would suggest create_thread. Then, this would match up almost exactly with the semantics of asyncio.create_task

create_task: Wrap the coroutine into a Task and schedule its execution. Return the Task object.

create_thread: Wrap the function in a thread and start its execution. Return a future object.

If you see what I mean? The functions are very similar but one is for coros concurrently in the event loop and the other is for functions concurrently in threads.

@cjerdonek
Copy link
Member

Thanks. @tmewett I think the proposed name is good enough. Re: create_thread(), I think it has the issue that when backed by a pool, it's not necessarily creating a new thread. With to_thread(), you can think of it as "going to" a thread in the pool.

Copy link
Contributor

@asvetlov asvetlov left a comment

Choose a reason for hiding this comment

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

I have a little doubt for the documentation text; the code itself and test are fine.

@1st1
Copy link
Member

1st1 commented May 18, 2020

@aeros Looks like everybody approves this so you can go ahead and merge!

Next todo:

  • Implement asyncio.from_thread
  • Implement auto-scaling of the default thread pool.
@aeros
Copy link
Contributor Author

aeros commented May 19, 2020

@aeros Looks like everybody approves this so you can go ahead and merge!

Awesome, thanks for the reviews @1st1, @cjerdonek, and @asvetlov!

I'll discuss the documentation a bit more w/ Andrew regarding the examples, and then proceed w/ merging it.

Co-authored-by: Andrew Svetlov <andrew.svetlov@gmail.com>
@aeros
Copy link
Contributor Author

aeros commented May 19, 2020

Thanks for the review as well @methane. :-)

@aeros
Copy link
Contributor Author

aeros commented May 19, 2020

Status check is done, and it's a failure x .

This occurred from a minor doctest warning, apparently having tildes in the code comments of example code is flagged as "suspicious". The latest commit should address it.

@miss-islington
Copy link
Contributor

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

@miss-islington miss-islington merged commit cc2bbc2 into python:master May 19, 2020
@zware
Copy link
Member

zware commented May 19, 2020

Note that this missed the 3.9 boat; it's now merged into 3.10. The whatsnew entry should be moved accordingly :)

(The documentation needs the appropriate .. versionadded: 3.10 marker, as well.)

@aeros
Copy link
Contributor Author

aeros commented May 19, 2020

Note that this missed the 3.9 boat; it's now merged into 3.10. The whatsnew entry should be moved accordingly :)

Currently, I'm reaching out to Łukasz to see if we could consider including this in 3.9b2 since it was so close to the deadline (it's been a WIP since last year and would have otherwise made it on-time, if not for some last minute changes). If he decides that it's too late, I'll update the whatsnew entry.

(I'll also add the versionadded marker as well, good catch.)

@miss-islington
Copy link
Contributor

Thanks @aeros for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-20212 is a backport of this pull request to the 3.9 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 19, 2020
Implements `asyncio.to_thread`, a coroutine for asynchronously running IO-bound functions in a separate thread without blocking the event loop. See the discussion starting from [here](https://github.com/python/cpython/pull/18410GH-issuecomment-628930973) in pythonGH-18410 for context. Automerge-Triggered-By: @aeros (cherry picked from commit cc2bbc2) Co-authored-by: Kyle Stanley <aeros167@gmail.com>
miss-islington added a commit that referenced this pull request May 19, 2020
Implements `asyncio.to_thread`, a coroutine for asynchronously running IO-bound functions in a separate thread without blocking the event loop. See the discussion starting from [here](https://github.com/python/cpython/pull/18410GH-issuecomment-628930973) in GH-18410 for context. Automerge-Triggered-By: @aeros (cherry picked from commit cc2bbc2) Co-authored-by: Kyle Stanley <aeros167@gmail.com>
@aeros
Copy link
Contributor Author

aeros commented May 19, 2020

Thanks, @ambv!

I'll try to open a PR to add the versionadded to the docs in the next couple of days, as @zware mentioned earlier.

miss-islington pushed a commit that referenced this pull request May 21, 2020
Allows contextvars from the main thread to be accessed in the separate thread used in `asyncio.to_thread()`. See the [discussion](#20143 (comment)) in GH-20143 for context. Automerge-Triggered-By: @aeros
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 21, 2020
…GH-20278) Allows contextvars from the main thread to be accessed in the separate thread used in `asyncio.to_thread()`. See the [discussion](https://github.com/python/cpython/pull/20143GH-discussion_r427808225) in pythonGH-20143 for context. Automerge-Triggered-By: @aeros (cherry picked from commit 0f56263) Co-authored-by: Kyle Stanley <aeros167@gmail.com>
miss-islington added a commit that referenced this pull request May 21, 2020
Allows contextvars from the main thread to be accessed in the separate thread used in `asyncio.to_thread()`. See the [discussion](https://github.com/python/cpython/pull/20143GH-discussion_r427808225) in GH-20143 for context. Automerge-Triggered-By: @aeros (cherry picked from commit 0f56263) Co-authored-by: Kyle Stanley <aeros167@gmail.com>
arturoescaip pushed a commit to arturoescaip/cpython that referenced this pull request May 24, 2020
Implements `asyncio.to_thread`, a coroutine for asynchronously running IO-bound functions in a separate thread without blocking the event loop. See the discussion starting from [here](python#18410 (comment)) in pythonGH-18410 for context. Automerge-Triggered-By: @aeros
arturoescaip pushed a commit to arturoescaip/cpython that referenced this pull request May 24, 2020
…GH-20278) Allows contextvars from the main thread to be accessed in the separate thread used in `asyncio.to_thread()`. See the [discussion](python#20143 (comment)) in pythonGH-20143 for context. Automerge-Triggered-By: @aeros
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet