Repository navigation
consider implementing __await__ on concurrent.futures.Future #68571
Description
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 = 'https://github.com/1st1'
closed_at = <Date 2016-12-16.21:37:12.452>
created_at = <Date 2015-06-04.15:38:14.031>
labels = ['type-feature', 'library', 'expert-asyncio']
title = 'consider implementing __await__ on concurrent.futures.Future'
updated_at = <Date 2016-12-16.21:37:12.451>
user = 'https://github.com/1st1'bugs.python.org fields:
activity = <Date 2016-12-16.21:37:12.451>
actor = 'yselivanov'
assignee = 'yselivanov'
closed = True
closed_date = <Date 2016-12-16.21:37:12.452>
closer = 'yselivanov'
components = ['Library (Lib)', 'asyncio']
creation = <Date 2015-06-04.15:38:14.031>
creator = 'yselivanov'
dependencies = []
files = ['39620', '39621', '39622', '39623', '39624', '40090', '40103', '40106']
hgrepos = []
issue_num = 24383
keywords = ['patch']
message_count = 39.0
messages = ['244832', '244835', '244837', '244838', '244839', '244895', '247270', '247271', '247786', '247788', '247804', '247806', '247807', '247812', '247813', '247817', '247820', '247867', '247868', '247869', '247871', '247874', '247877', '247879', '247882', '247886', '247891', '247892', '247923', '247925', '247928', '247931', '247935', '248200', '248204', '248209', '248210', '248932', '283443']
nosy_count = 7.0
nosy_names = ['gvanrossum', 'scoder', 'vstinner', 'alex.gronholm', 'Yury.Selivanov', 'yselivanov', 'emptysquare']
pr_nums = []
priority = 'normal'
resolution = 'wont fix'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue24383'
versions = ['Python 3.6']
Activity
1st1 commentedon Jun 4, 2015
It didn't occur to me that we can implement __await__ on concurrent.Future to integrate it *with* asyncio. I guess it makes sense (although we don't have this kind of integration in 3.4 with 'yield from')
It is -- we just have to return "iter(asyncio.Future())"
Please see the attached patch -- it needs some more tuning, but it demonstrates that the integration is possible.
1st1 commentedon Jun 4, 2015
Added a unittest for cancellation
1st1 commentedon Jun 4, 2015
Alternative patch with monkeypatching instead of Future subclassing.
gvanrossum commentedon Jun 4, 2015
Sorry, I don't like that either. See my review.
gvanrossum commentedon Jun 4, 2015
Thinking about this more I think we should pass on this for now.
scoder commentedon Jun 6, 2015
(Copying my review comment here so that it doesn't get lost as it's more of a general design thing than a specific review comment.)
Having Future know and deal with asyncio seems wrong to me. asyncio should be able to deal *somehow* with a Future that is being awaited, but a Future shouldn't require asyncio in order to be awaited.
agronholm commentedon Jul 24, 2015
Yes, Yury's approach is wrong here -- Futures should not know about asyncio, but asyncio should be able to handle Futures natively. This seems like the obvious solution to me. Any counterarguments?
YurySelivanov commentedon Jul 24, 2015
There are no counterarguments. There is no obvious way to support concurrent.futures transparently, though:
await conc_fut
requires conc_fut to implement __await__.
So we either have to implement __await__ for concurrent futures and provide some kind of registry for frameworks, or we can implement a wrapper function:
Anyways, concrete ideas and API suggestions are welcome.
agronholm commentedon Jul 31, 2015
Sorry to keep you waiting. This sample code runs fine with the attached patch: https://gist.github.com/agronholm/43c71be0028bb866753a
In short, I implemented support for concurrent.futures in the asyncio Task class instead of making concurrent.futures aware of asyncio. The __await__ implementation in concurrent.futures closely mirrors that of asyncio's Future.
1st1 commentedon Jul 31, 2015
I like your approach and the proposed patch. I think we should commit this in 3.6 (the final patch should include docs & tests).
vstinner commentedon Aug 1, 2015
I'm not sure that it's a good idea to start adding special cases in Task
class which is already very complex.
Instead we may add a way to register custom awaitable objects?
15 remaining items
agronholm commentedon Aug 2, 2015
Updated patch per review comments.
I also corrected the order of the lines in the new block. If _must_cancel is True, it would have tried to call cancel() on _fut_waiter before it was set. Now the code matches that of the original block.
The docs don't seem to explicitly say anywhere that they only accept asyncio's Futures and not concurrent Futures, so I'm unsure if any changes are needed there at all.
scoder commentedon Aug 2, 2015
There are a couple of use cases in the context of asyncio, but definitely
also a major one: blocking database drivers. Normally, applications keep a
pool of connections open to a database and use a bounded size for it in
order to limit the resource usage on server side. Starting a thread pool of
the same size in the application provides a trivial way to make use of
these connections concurrently and efficiently with a blocking database
driver and/or ORM (e.g. SQLAlchemy). "concurrent.futures" makes this really
easy.
concurrent.futures.Future is not just any Future, it's the one that is in
the standard library, i.e. pretty much the only one that every tool could
agree on (whether or not it knows about or uses asyncio). The fact that
it's thread-safe doesn't really matter to me.
agronholm commentedon Aug 3, 2015
+1. It was specifically SQLAlchemy (but not limited to it -- there are many other blocking APIs) that made me look for a way to easily use threads with native coroutines.
The best workaround I've come up with:
But as I mentioned before, wrap_future() is nowhere to be found in the asyncio docs.
agronholm commentedon Aug 3, 2015
Created bpo-24785 for the documentation of wrap_future().
gvanrossum commentedon Aug 3, 2015
Every individual use case can be dealt with easily by adding simple helper
functions. I really want to keep async and threads separate. And it's no
coincidence that concurrent.futures is threadsafe; that's part of its spec.
agronholm commentedon Aug 3, 2015
Why do you want to keep threads separate from asyncio? What's the downside here?
The use of helper functions is justifiable when integrating a third party framework or similar with asyncio, but standard library components should just integrate well with each other. Requiring boilerplate wrappers for such use seems unreasonable to me.
gvanrossum commentedon Aug 3, 2015
Sorry, there's no space in this issue for an answer.
agronholm commentedon Aug 7, 2015
Where do we stand with this then? Should I start a thread on python-dev to get the ball rolling?
1st1 commentedon Aug 7, 2015
Alex, sure, go ahead. Although I think python-ideas would be a better choice.
gvanrossum commentedon Aug 7, 2015
You could also withdraw. The more I think about it the more I dislike it. I just don't think we should do *anything* that encourages confusion between threads and tasks. They are fundamentally different concepts and should remain so.
agronholm commentedon Aug 7, 2015
I've already made my case on python-ideas, so let's talk it over there.
agronholm commentedon Aug 21, 2015
I've implemented my background-calling code in my framework via run_in_executor() now, so this has become a non-issue for me. I have no more interest in this patch.
1st1 commentedon Dec 16, 2016
I think we should close this one. Making concurrent.futures awaitable will likely only complicate things for end-users.