New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability to create requests with session context without sending immedately #1445

Closed
robcowie opened this Issue Jul 8, 2013 · 10 comments

Comments

Projects
None yet
4 participants
@robcowie

robcowie commented Jul 8, 2013

I've just been scratching my head whilst trying to figure out how to create a Request() instance independently of a session and have that session 'prepare' it by augmenting it with the session context (cookies etc.) before sending it. Looks like it isn't possible without custom transports (described in #1280)

My use case is thus...

  1. My code is provided with a session, constructed elsewhere. Typically it has auth cookies.
  2. One of several functions that build and return an unprepared request instance are selected at run time and called. The request is then potentially manipulated by yet more code, also determined by run-time conditions.
  3. The request is prepared - in the context of the existing session - and sent.

Currently this is not possible. I am required to manually add cookies and other session-borne context to the request before preparing it and passing to session.send(). It therefore seems difficult to separate session creation and request creation.

It would be nice to have some way of providing an unprepared request to a session and have the session prepare it. Something like...

response = Session().send(unprepared_request)
# or...
s = Session()
request = s.prepare(unprepared_request)
s.send(request)
# or...
Session().request(method, uri, return_response=true, **kargs)

It appears that similar requests have been declined in the past. I'd like to re-open the discussion if possible. If there is still no appetite for it, I'll have to maintain a fork I guess. Rather that than complicate matters with custom transports.

@Lukasa

This comment has been minimized.

Show comment
Hide comment
@Lukasa

Lukasa Jul 8, 2013

Member

Note: I deleted your other comment to keep this all in once place.

Custom transports won't help, they don't have easy access to the Session object. I'm sure some Python hackery could be performed to give it to them, but it's kind of insane, that's not what they're for. The custom adapter in the linked issue was to solve that specific issue, not in general.

I'm not 100% opposed to this, I'm really not. I think a small refactor would be needed to break up the logic of Session.request(), but that would be plenty do-able. What I am not sold on is whether there is generally a use-case for building an unprepared Request yourself but then wanting to pass it through a Session. Do you mind elaborating on your use-case?

Member

Lukasa commented Jul 8, 2013

Note: I deleted your other comment to keep this all in once place.

Custom transports won't help, they don't have easy access to the Session object. I'm sure some Python hackery could be performed to give it to them, but it's kind of insane, that's not what they're for. The custom adapter in the linked issue was to solve that specific issue, not in general.

I'm not 100% opposed to this, I'm really not. I think a small refactor would be needed to break up the logic of Session.request(), but that would be plenty do-able. What I am not sold on is whether there is generally a use-case for building an unprepared Request yourself but then wanting to pass it through a Session. Do you mind elaborating on your use-case?

@robcowie

This comment has been minimized.

Show comment
Hide comment
@robcowie

robcowie Jul 8, 2013

cheers for the delete. I intended to do it... but I'm on a bus right now.

Briefly, my use case is a code framework which provides users with the ability to provide functions to - amongst other things - build a session (usually requests but not always), and build requests given a set of parameters (usually just a url).

The framework then uses these functions to repeatedly build requests and ultimately hand them off to the session for dispatch.

Between each of these steps, other non-http things happen. Basically, as it exists now the framework user has the ability to define how the requests and the session are built, each in isolation.

I'll try to provide some simplified code examples later.

robcowie commented Jul 8, 2013

cheers for the delete. I intended to do it... but I'm on a bus right now.

Briefly, my use case is a code framework which provides users with the ability to provide functions to - amongst other things - build a session (usually requests but not always), and build requests given a set of parameters (usually just a url).

The framework then uses these functions to repeatedly build requests and ultimately hand them off to the session for dispatch.

Between each of these steps, other non-http things happen. Basically, as it exists now the framework user has the ability to define how the requests and the session are built, each in isolation.

I'll try to provide some simplified code examples later.

@robcowie

This comment has been minimized.

Show comment
Hide comment
@robcowie

robcowie Jul 9, 2013

This isn't my actual code but shows the use case

from mylib import Importer

def request_factory(uri):
    return requests.Request('GET', uri)

def session_with_cookie(**kargs):
    sess = requests.Session()
    sess.get('http://httpbin.org/cookies/set?k1=v1')
    return sess

config = {'uris': [], 'a': 'a'}
imp = Importer(request=request_factory, session=session_with_cookie)
imp.run(config)

On imp.run() a request is created for each supplied url, and sent with the session created by session_with_cookie.

robcowie commented Jul 9, 2013

This isn't my actual code but shows the use case

from mylib import Importer

def request_factory(uri):
    return requests.Request('GET', uri)

def session_with_cookie(**kargs):
    sess = requests.Session()
    sess.get('http://httpbin.org/cookies/set?k1=v1')
    return sess

config = {'uris': [], 'a': 'a'}
imp = Importer(request=request_factory, session=session_with_cookie)
imp.run(config)

On imp.run() a request is created for each supplied url, and sent with the session created by session_with_cookie.

@Lukasa

This comment has been minimized.

Show comment
Hide comment
@Lukasa

Lukasa Jul 9, 2013

Member

Mm, I guess I can see wanting to do this. I'm still pretty much +0 on it though, so I'll defer to @sigmavirus24 and @kennethreitz.

Member

Lukasa commented Jul 9, 2013

Mm, I guess I can see wanting to do this. I'm still pretty much +0 on it though, so I'll defer to @sigmavirus24 and @kennethreitz.

@robcowie

This comment has been minimized.

Show comment
Hide comment
@robcowie

robcowie Jul 10, 2013

If the answer is no, I'd still be interested to hear your ideas about how you would implement it.

robcowie commented Jul 10, 2013

If the answer is no, I'd still be interested to hear your ideas about how you would implement it.

@Lukasa

This comment has been minimized.

Show comment
Hide comment
@Lukasa

Lukasa Jul 10, 2013

Member

How I would implement it is easy: I'd add a new Session.build_request() function to the Session object, and put the Request-specific portions of the setup in there, then call that from Session.request().

Member

Lukasa commented Jul 10, 2013

How I would implement it is easy: I'd add a new Session.build_request() function to the Session object, and put the Request-specific portions of the setup in there, then call that from Session.request().

@sigmavirus24

This comment has been minimized.

Show comment
Hide comment
@sigmavirus24

sigmavirus24 Jul 13, 2013

Member

I'm all for breaking some stuff into a separate method to make testing in the future easier. Whether we want to make that part of the public API, I don't know.

Member

sigmavirus24 commented Jul 13, 2013

I'm all for breaking some stuff into a separate method to make testing in the future easier. Whether we want to make that part of the public API, I don't know.

@sigmavirus24

This comment has been minimized.

Show comment
Hide comment
@sigmavirus24

sigmavirus24 Jul 19, 2013

Member

Well with a possible 2.0 this is possible.

Member

sigmavirus24 commented Jul 19, 2013

Well with a possible 2.0 this is possible.

@erydo

This comment has been minimized.

Show comment
Hide comment
@erydo

erydo Jul 31, 2013

Contributor

I actually want this too.

To clarify the use case, I expect to be able to do something like this.

class FooApiRequests(object):
    def __init__(self, server_url='http://example.com'):
         self.server_url = server_url

    def foo(self):
         return Request('GET', self.server_url + '/foo')

requests = FooApiRequests()
session = Session(auth=SomeAuthThing())

req = requests.foo()
session.send(req.prepare())

However, Session.send doesn't apply its configuration (e.g. auth) that way. Essentially, I'd like the "thing that creates request objects" independent of the Session.

In our real code, FooApiRequests represents a larger API request-factory creating many kinds of requests; and whose requests may get reused elsewhere for things like automated-retries-after-auth, etc.

I'd be basically okay doing this:

class FooApiRequests(object):
    def __init__(self, session, server_url='http://example.com'):
         self.session = session
         self.server_url = server_url

    def foo(self):
         return self.session.build_request('GET', self.server_url + '/foo')

session = Session(auth=SomeAuthThing())
requests = FooApiRequests(session=session)

req = requests.foo()
session.send(req.prepare())

but I think this would be even better:

class FooApiRequests(object):
    def __init__(self, server_url='http://example.com'):
         self.server_url = server_url

    def foo(self):
         return Request('GET', self.server_url + '/foo')

session = Session(auth=SomeAuthThing())
requests = FooApiRequests()

req = requests.foo()
prepared_request = session.prepare_request(req)
session.send(prepared_request)

In which Session.prepare_request() creates a PreparedRequest whose settings have been merged with the session's.

I'd be okay making that change and submitting a PR if it seems like a good solution. It's a non-breaking API change. @sigmavirus24 and @Lukasa, if I were to go ahead with that, would it be likely to be merged in? Any suggestions?

Contributor

erydo commented Jul 31, 2013

I actually want this too.

To clarify the use case, I expect to be able to do something like this.

class FooApiRequests(object):
    def __init__(self, server_url='http://example.com'):
         self.server_url = server_url

    def foo(self):
         return Request('GET', self.server_url + '/foo')

requests = FooApiRequests()
session = Session(auth=SomeAuthThing())

req = requests.foo()
session.send(req.prepare())

However, Session.send doesn't apply its configuration (e.g. auth) that way. Essentially, I'd like the "thing that creates request objects" independent of the Session.

In our real code, FooApiRequests represents a larger API request-factory creating many kinds of requests; and whose requests may get reused elsewhere for things like automated-retries-after-auth, etc.

I'd be basically okay doing this:

class FooApiRequests(object):
    def __init__(self, session, server_url='http://example.com'):
         self.session = session
         self.server_url = server_url

    def foo(self):
         return self.session.build_request('GET', self.server_url + '/foo')

session = Session(auth=SomeAuthThing())
requests = FooApiRequests(session=session)

req = requests.foo()
session.send(req.prepare())

but I think this would be even better:

class FooApiRequests(object):
    def __init__(self, server_url='http://example.com'):
         self.server_url = server_url

    def foo(self):
         return Request('GET', self.server_url + '/foo')

session = Session(auth=SomeAuthThing())
requests = FooApiRequests()

req = requests.foo()
prepared_request = session.prepare_request(req)
session.send(prepared_request)

In which Session.prepare_request() creates a PreparedRequest whose settings have been merged with the session's.

I'd be okay making that change and submitting a PR if it seems like a good solution. It's a non-breaking API change. @sigmavirus24 and @Lukasa, if I were to go ahead with that, would it be likely to be merged in? Any suggestions?

@Lukasa

This comment has been minimized.

Show comment
Hide comment
@Lukasa

Lukasa Aug 15, 2013

Member

Resolved by #1507. =)

Member

Lukasa commented Aug 15, 2013

Resolved by #1507. =)

@Lukasa Lukasa closed this Aug 15, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment