loop.run_in_executor should propagate current contextvars

[hellysmile]

BPO	34014
Nosy	@asvetlov, @1st1, @hellysmile
PRs	bpo-34014: Added support of contextvars for BaseEventLoop.run_in_executor #8035 bpo-34014: Added support of contextvars for BaseEventLoop.run_in_executor #9688

^{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:

assignee = None
closed_at = <Date 2022-03-23.20:57:22.475>
created_at = <Date 2018-06-30.23:26:24.700>
labels = ['type-feature', '3.9', 'expert-asyncio']
title = 'loop.run_in_executor should propagate current contextvars'
updated_at = <Date 2022-03-23.20:57:22.474>
user = 'https://github.com/hellysmile'

bugs.python.org fields:

activity = <Date 2022-03-23.20:57:22.474>
actor = 'asvetlov'
assignee = 'none'
closed = True
closed_date = <Date 2022-03-23.20:57:22.475>
closer = 'asvetlov'
components = ['asyncio']
creation = <Date 2018-06-30.23:26:24.700>
creator = 'hellysmile'
dependencies = []
files = []
hgrepos = []
issue_num = 34014
keywords = ['patch']
message_count = 14.0
messages = ['320814', '320817', '320818', '320819', '320820', '326812', '326813', '326950', '326972', '326975', '326978', '326981', '350467', '415908']
nosy_count = 3.0
nosy_names = ['asvetlov', 'yselivanov', 'hellysmile']
pr_nums = ['8035', '9688']
priority = 'normal'
resolution = 'out of date'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue34014'
versions = ['Python 3.9']

[hellysmile]

PEP-567 supports copying context into another threads, but for asyncio users each call run_in_executor requires explicit context propagation

For end users expected behavior that context is copied automatically

[1st1]

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".

[1st1]

As a workaround you can subclass the ThreadPoolExecutor and set it as a default executor.

[hellysmile]

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 Context variables are natively supported in asyncio is hard for end users.

Each use case of contextvars + run_in_executor will produce copy-paste boilerplate

As for me it is very similar to the asyncio.get_event_loop() history

[hellysmile]

ProcessPoolExecutor as executor is not so popular in real world - as executor for asyncio

[1st1]

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?

[1st1]

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.

[asvetlov]

I vote on not changing run_in_executor behavior if we cannot make it work with ProcessPoolExecutor.

If a new API will solve the problem -- that's fine.
Until it landed the explicit context propagation is the satisfactory solution.

[hellysmile]

I've created new pull request #9688 with the implementation of proposed changes

[1st1]

[Andrew]

I vote on not changing run_in_executor behavior if we cannot make it work with ProcessPoolExecutor.

If a new API will solve the problem -- that's fine.
Until it landed the explicit context propagation is the satisfactory solution.

I'm not sure I understand. Should we close this issue or you're OK with (3)?

[asvetlov]

I prefer (4) but can live with (3)

It is a new feature for Python 3.8 anyway, no need to rush

[1st1]

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).