loop.run_in_executor should be a coroutine#306
loop.run_in_executor should be a coroutine#306Martiusweb wants to merge 1 commit intopython:masterpython/asyncio:masterfrom Martiusweb:run_in_executor_coroMartiusweb/asyncio:run_in_executor_coroCopy head branch name to clipboard
Conversation
Since base_events.BaseEventLoop.run_in_executor returns a Future object, a caller can call it with yield from/await. However, the result of the call is not a coroutine since asyncio.iscoroutine(loop.run_in_executor(...)) returns False. It matters when one wants to use run_in_executor() in a task, such as: loop.create_task(loop.run_in_executor(...)) In this case, an exception is raised immediatly, while the task is effectively running in the executor. This patch propose to make loop.run_in_executor() be an actual coroutine function (and be tested). I believe that returning a Future and document the function as a coroutine used to be done quite often, maybe this should be fixed elsewhere too. An alternative solution would be to change the documentation, explain why it can not be used with loop.create_task() and should be used with loop.ensure_future() instead.
|
There are too many places inside asyncio where coroutine is technically a regular function returning Future instance. |
|
I don't think it's perfectly fine: it creates an inconsistency between coroutines and other awaitable objects which leads to confusion for the user and subtle bugs. Most of the time, There are subtle differences between a function returning a future and a coroutine. For instance, a coroutine is executed only when awaited on (i.e. |
|
You've really got this backwards. The most general concept here is not the Task but the Future (which is a base class of Task). If you have either a coroutine or a Future and you want a unified API, you should call ensure_future(), not create_task(). If you already have a Future there's no point in trying to wrap it into a Task, nor to try to make a coroutine into it -- that's backwards. There is no philosophical difference between Task/coroutine and Future in the way you assume. If you really have a use case where it matters to you that the action doesn't get started until the loop runs (which seems suspicious to me) you will have to implement this in some other way. |
Since base_events.BaseEventLoop.run_in_executor returns a Future object, a caller can call it with yield from/await. However, the result of the call is not a coroutine since asyncio.iscoroutine(loop.run_in_executor(...)) returns False.
It matters when one wants to use run_in_executor() in a task, such as:
loop.create_task(loop.run_in_executor(...))
In this case, an exception is raised immediatly (AssertionError), while the task is effectively running in the executor.
This patch propose to make loop.run_in_executor() be an actual coroutine function (and be tested).
I believe that returning a Future and document the function as a coroutine used to be done quite often, maybe this should be fixed elsewhere too.
An alternative solution would be to change the documentation, explain why it can not be used with loop.create_task() and should be used with loop.ensure_future() instead.