ross / requests-futures

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

Use an existing session object in FuturesSesion #28

Closed dgouldin closed 8 years ago

dgouldin commented 8 years ago

Add a keyword argument to FuturesSession which will allow an instance to use an existing session object rather than deferring to its superclass when making a request.

Resolves #27.

ross commented 8 years ago

Code seems reasonable. Main question was the need, but I see the reasoning in #27. I have half baked thoughts about other ways to solve this like moving the bulk of the logic to a mixin, something like:

class FuturesMixin(object):
    ...

class FuturesSession(FuturesMixin, Session):
    ...

That would allow you to create a subclass of OAuth2Session and include the mixin first as well. Not sure if that's cleaner than this. Thoughts? I'm ok with :ship:ing this, but just wanted to run :arrow_up: by you first to get your thoughts.

dgouldin commented 8 years ago

I'm always a bit wary of multiple inheritance in python, especially considering both FuturesMixin and OAuth2Session would override request:

https://github.com/requests/requests-oauthlib/blob/master/requests_oauthlib/oauth2_session.py#L307

This solution is less composable but also simpler IMO.

ross commented 8 years ago

:cool: