classification
Title: AbstactEventLoop.run_in_executor is listed as an async method, but should actually return a Future
Type: behavior Stage: resolved
Components: asyncio, Library (Lib) Versions: Python 3.10, Python 3.9, Python 3.8, Python 3.7, Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: aeros, asvetlov, gvanrossum, jamesba, miss-islington, yselivanov
Priority: normal Keywords: patch

Created on 2020-05-26 15:45 by jamesba, last changed 2020-08-17 14:41 by gvanrossum. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 21852 merged jamesba, 2020-08-13 07:36
PR 21903 merged miss-islington, 2020-08-17 14:21
PR 21904 merged miss-islington, 2020-08-17 14:21
Messages (8)
msg370005 - (view) Author: James Barrett (jamesba) * Date: 2020-05-26 15:45
As discussed in < https://github.com/python/typeshed/issues/3999#issuecomment-634097968 > the type of `AbstractEventLoop.run_in_executor` is defined at < https://github.com/python/cpython/blob/master/Lib/asyncio/events.py#L286 > as follows:

```
async def run_in_executor(self, executor, func, *args):
        raise NotImplementedError
```

However all concrete implementations of this method are actually not async methods but rather synchronous methods which return a Future object.

Logically this appears to make sense: at base `run_in_executor` is not a coroutine, since it doesn't create an object representing code which will be executed when the object is awaited, rather it returns an object representing code which is running asynchronously elsewhere (on another thread) and which can be awaited to wait for that other thread to complete its task. Which seems to be a perfect match to what a Future object is supposed to be.

As such it seems that the current definition of the method as a coroutine is possibly a mistake.

Alternatively if some feel that it is important to allow concrete implementations to implement it as a coroutine if they need to then perhaps it could be specified to be a method returning an Awaitable, since that would cover both options?
msg370045 - (view) Author: Kyle Stanley (aeros) * (Python committer) Date: 2020-05-27 03:37
From looking at the commit history of AbstactEventLoop.run_in_executor(), it seems that it was previously be a non-coroutine method prior to the conversion from the `@asyncio.coroutine` decorator to `async def` (PR-4753). See https://github.com/python/cpython/blame/ede157331b4f9e550334900b3b4de1c8590688de/Lib/asyncio/events.py#L305.

The only context for the change I can find is the following conversation between Andrew and Yury: https://github.com/python/cpython/pull/4753#issuecomment-350114336. However, the example provided of `connect_read_pipe()` had already been a coroutine at the time for the BaseEventLoop implementation, which makes sense in that case. So, it's not clear to me as to why `run_in_executor()` was also converted to "async def" when its main implementation is not a coroutine. Furthermore, it's documented as an awaitable rather than a coroutine (https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.loop.run_in_executor).

@Andrew do you have any additional context to provide that I'm potentially missing?
msg372735 - (view) Author: James Barrett (jamesba) * Date: 2020-07-01 08:03
Is there any further movement on this?
msg375278 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2020-08-12 18:25
I think it makes sense to remove the `async` from the definition in AbstractEventLoop.

If you want to help, you can submit a PR to do it.
msg375375 - (view) Author: James Barrett (jamesba) * Date: 2020-08-14 07:48
https://github.com/python/cpython/pull/21852
msg375548 - (view) Author: miss-islington (miss-islington) Date: 2020-08-17 14:20
New changeset 29f84294d88ec493c2de9d6e8dbc12fae3778771 by James Weaver in branch 'master':
bpo-40782: Change asyncio.AbstractEventLoop.run_in_executor to be a method not a coroutine (GH-21852)
https://github.com/python/cpython/commit/29f84294d88ec493c2de9d6e8dbc12fae3778771
msg375551 - (view) Author: miss-islington (miss-islington) Date: 2020-08-17 14:37
New changeset 1baa8b14ee23ef3040923f53565c8d1bafd28117 by Miss Islington (bot) in branch '3.8':
bpo-40782: Change asyncio.AbstractEventLoop.run_in_executor to be a method not a coroutine (GH-21852)
https://github.com/python/cpython/commit/1baa8b14ee23ef3040923f53565c8d1bafd28117
msg375552 - (view) Author: miss-islington (miss-islington) Date: 2020-08-17 14:40
New changeset d6bdf6d52f0400df1bd1dce24aaad9514015c755 by Miss Islington (bot) in branch '3.9':
bpo-40782: Change asyncio.AbstractEventLoop.run_in_executor to be a method not a coroutine (GH-21852)
https://github.com/python/cpython/commit/d6bdf6d52f0400df1bd1dce24aaad9514015c755
History
Date User Action Args
2020-08-17 14:41:46gvanrossumsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2020-08-17 14:40:33miss-islingtonsetmessages: + msg375552
2020-08-17 14:37:32miss-islingtonsetmessages: + msg375551
2020-08-17 14:21:30miss-islingtonsetpull_requests: + pull_request21021
2020-08-17 14:21:24miss-islingtonsetpull_requests: + pull_request21020
2020-08-17 14:20:06miss-islingtonsetnosy: + miss-islington
messages: + msg375548
2020-08-14 08:40:46ned.deilysetcomponents: + asyncio
2020-08-14 07:48:16jamesbasetmessages: + msg375375
2020-08-13 07:49:27serhiy.storchakasettitle: AbstactEventLoop.run_in_executor is listed as an async method, but should actually return a Futrue -> AbstactEventLoop.run_in_executor is listed as an async method, but should actually return a Future
2020-08-13 07:36:18jamesbasetkeywords: + patch
stage: patch review
pull_requests: + pull_request20979
2020-08-12 18:25:44gvanrossumsetnosy: + gvanrossum
messages: + msg375278
2020-07-01 08:03:53jamesbasetmessages: + msg372735
2020-05-27 03:37:08aerossetnosy: + asvetlov, yselivanov, aeros
messages: + msg370045
2020-05-26 15:45:38jamesbacreate