Verbose TypeError for asyncio.ensure_future

[crenwick]

BPO	30098
Nosy	@bitdancer, @1st1, @Mariatta, @crenwick
PRs	bpo-30098: Adds verbose error to asyncio.ensure_future #1170 [3.5] bpo-30098: Clarify that run_coroutine_threadsafe expects asynci… #1246 [3.6] bpo-30098: Clarify that run_coroutine_threadsafe expects asynci… #1247

^{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 2017-04-22.03:00:36.045>
created_at = <Date 2017-04-18.22:24:34.208>
labels = ['type-feature', '3.7', 'expert-asyncio']
title = 'Verbose TypeError for asyncio.ensure_future'
updated_at = <Date 2017-04-22.03:00:36.043>
user = 'https://github.com/crenwick'

bugs.python.org fields:

activity = <Date 2017-04-22.03:00:36.043>
actor = 'Mariatta'
assignee = 'none'
closed = True
closed_date = <Date 2017-04-22.03:00:36.045>
closer = 'Mariatta'
components = ['asyncio']
creation = <Date 2017-04-18.22:24:34.208>
creator = 'crenwick'
dependencies = []
files = []
hgrepos = []
issue_num = 30098
keywords = []
message_count = 14.0
messages = ['291845', '291846', '291847', '291848', '291849', '291850', '291851', '291852', '291856', '291885', '292077', '292096', '292097', '292098']
nosy_count = 4.0
nosy_names = ['r.david.murray', 'yselivanov', 'Mariatta', 'crenwick']
pr_nums = ['1170', '1246', '1247']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue30098'
versions = ['Python 3.5', 'Python 3.6', 'Python 3.7']

[crenwick]

Despite the shy mention in the docs, it was not clear to me that the future returned from asyncio.run_coroutine_threadsafe is not compatible with asyncio.ensure_future (and other asyncio functions), and it took me a fair amount of frustration and source-code-digging to figure out what was going on.

To avoid this confusion for other users, I think that a verbose TypeError warning when a concurrent.futures.Future object is passed into asyncio.ensure_future would be very helpful.

[1st1]

asyncio.run_coroutine_threadsafe was't designed to be compatible with asyncio.ensure_future. It should be used to run a coroutine in an event loop from another thread. You shouldn't need to await on the future it returns.

[crenwick]

Totally. But this wasn't immediately clear to me when using the API. A verbose error like this one would have made this behavior more clear to me.

[1st1]

A verbose error like this one would have made this behavior more clear to me.

Well, it already raises a TypeError with a clear message -- this is how we tell the user that the argument type is not supported in Python. I don't think we need to specialize this code any further.

Instead I'd suggest to try to improve asyncio documentation for ensure_future and run_coroutine_threadsafe.

[crenwick]

it already raises a TypeError with a clear message

This is more to my point: I found the TypeError message not clear at all.

From my prospective, I was using a future object returned from an asyncio function to call another asyncio function, yet that function was telling me it could only use a future with it.

Though I eventually figured out what was going on, I think that a more clear message at that moment would have been very helpful to me. And I think other users who benefit from this was well.

A better warning/wording for the API would be helpful as well, but I don't think it fully suffices the problem.

[1st1]

This is more to my point: I found the TypeError message not clear at all.

From my prospective, I was using a future object returned from an asyncio function to call another asyncio function, yet that function was telling me it could only use a future with it.

I understand, but this is the first time I see somebody tries to use ensure_future on run_coroutine_threadsafe result, which suggests that it's not a common problem.

Specializing error messages makes code more complex and adds to maintenance overhead, so we can't afford to make changes on first request. In cases like this we usually put the issue on hold to see if more people complain.

[crenwick]

Totally understandable. Some final thoughts:

this is the first time I see somebody tries to use ensure_future on run_coroutine_threadsafe result, which suggests that it's not a common problem.

• I wonder how much self-reporting will be done here.

• I also considered making the existing error message more verbose (I think that still could be effective). However, I decided to open this PR as a separate TypeError to not muddle the language of the probably more common TypeError.

Thanks for your time and insight, Yury!

[1st1]

• I wonder how much self-reporting will be done here.

Yeah, I don't think we see enough reporting on bugs.python.org. I hope people will become more vocal :) I try to monitor what people say about asyncio on SO, Twitter and /r/python to get a better picture.

• I also considered making the existing error message more verbose (I think that still could be effective).

Feel free to suggest a better wording.

[bitdancer]

I don't think your specialized error message adds anything. The the most common mistake, IMO, is going to be not realizing that run_coroutine_threadsafe don't return one of the acceptable types. So being told that concurrent.future.Future is not acceptable will add zero additional information. I think the only change that needs to be made is to make the existing message say 'asyncio.Future' instead of just 'Future'.

[crenwick]

Feel free to suggest a better wording.

I think the only change that needs to be made is to make the existing message say 'asyncio.Future' instead of just 'Future'.

I hear ya. New commit in the PR reverts the TypeError I added, and changes the original TypeError language to simply read "An asyncio.Future ..." instead of just "A Future ...".

[Mariatta]

New changeset ae5b326 by Mariatta (Charles Renwick) in branch 'master':
bpo-30098: Clarify that run_coroutine_threadsafe expects asyncio.Future (GH-1170)
ae5b326

[Mariatta]

New changeset 503d74a by Mariatta in branch '3.5':
bpo-30098: Clarify that run_coroutine_threadsafe expects asyncio.Future (GH-1170) (bpo-1246)
503d74a

[Mariatta]

New changeset a3d8dda by Mariatta in branch '3.6':
bpo-30098: Clarify that run_coroutine_threadsafe expects asyncio.Future (GH-1170) (bpo-1247)
a3d8dda

[Mariatta]

PR has been merged and backported to 3.5 and 3.6.
Thanks everyone :)