-
-
Notifications
You must be signed in to change notification settings - Fork 31k
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
loop.run_in_executor should propagate current contextvars #78195
Comments
PEP-567 supports copying context into another threads, but for asyncio users each call For end users expected behavior that context is copied automatically |
I considered enabling that, but in the end decided not to. The reason is that it's possible to use a ProcessPoolExecutor with run_in_execuror(), and context cars currently don't support pickling (and probably never will). We can't have a single api that sometimes works with contextvars and sometimes doesn't. So this is a "won't fix". |
As a workaround you can subclass the ThreadPoolExecutor and set it as a default executor. |
What about to check instance of executor and depending on that propagate contextvars or not? As well to raise RuntimeError for ProcessPoolExecutor with provided context? I assume, that ProcessPoolExecutor as executor is not so popular in real world, but I can not provide any stats. Different behavior is defiantly at least weird, but do not support natively what is technically expected also strange. https://docs.python.org/3.7/library/contextvars.html#asyncio-support Each use case of contextvars + run_in_executor will produce copy-paste boilerplate As for me it is very similar to the |
|
So we're deprecating passing non-ThreadPoolExecutor instances to loop.set_default_executor. In 3.9 that will trigger an error. For this issue we have basically the next few options: (1) Do nothing; (2) Fix "run_in_executor" to start copying and running in correct context automatically (3) Add a new keyword-only parameter to "run_in_executor": "retain_context=False" (4) Design a new async/await friendly API for using thread- and process-pools. Now, (4) will happen. The "run_in_executor" method is low-level and requires accessing the event loop to use it. We probably have enough time to design this new API before 3.8. As for implementing (3) in 3.8 -- I'd be OK with that too. Andrew, your thoughts? |
One problem with (3) is what will happen if someone uses "retain_context=True" and a ProcessPoolExecutor. It has to fail in a graceful and obvious way. |
I vote on not changing If a new API will solve the problem -- that's fine. |
I've created new pull request #9688 with the implementation of proposed changes |
[Andrew]
I'm not sure I understand. Should we close this issue or you're OK with (3)? |
I prefer (4) but can live with (3) It is a new feature for Python 3.8 anyway, no need to rush |
Yep, I agree. Let's see if we end up having a new nice high-level API in 3.8; if not we go for (3). |
Hey, as I see there is no new API yet. Can we take a look again on 3) ? I'll be happy to update PR 9688 |
contextvars and run_in_executor() cannot be used together with process executor. asyncio.to_thread() runs with a copy of the current context. https://docs.python.org/3/library/asyncio-task.html?highlight=to_thread#asyncio.to_thread |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: