ross / requests-futures

Asynchronous Python HTTP Requests for Humans using Futures
Other
2.11k stars 152 forks source link

Also make Session.send concurrent #21

Closed alexjg closed 9 years ago

ross commented 9 years ago

ah, wasn't familiar with the send api. happy to take a closer look and merge the PR, but needs some unit testing.

alexjg commented 9 years ago

I did this in a hurry last night, there are some holes in it. I'll update this PR this weekend with more tests.

ross commented 9 years ago

just took a look at this myself, it's actually not cleanly possible. Session.request uses Session.send as part of it's work. if futures support is added to FuturesSession.send things get double wrapped. if if try to add it only to FuturesSession.send then Session.request complains about the background_callback parameter b/c it has an explicit list of supported params.

going to dig a bit more, but this isn't going to be quickly/easily possible to resolve.

ross commented 9 years ago

don't know how i missed it in the past, but there's a hooks param to Request that is almost exactly the same functionally as background_callback. actually it's a bit more powerful. i never would have added background_callback if i knew that exiected.

at first i thought i'd be able to make use of the hooks param to work around the problems with moving the async to send, but unfortunately send is used in various places internally that prevent me from being able to return an async object from it, namely redirect processing.

so it's just not possible to make send async given the current structure/api of requests :(

if when requests itself is interested in addressing the problem we can revisit.