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 prefix session URLs #2554

Closed
foxx opened this Issue Apr 21, 2015 · 10 comments

Comments

Projects
None yet
3 participants
@foxx

foxx commented Apr 21, 2015

It would be nice to be able to prefix session URLs to avoid repetitive code.

Consider the following code;

from requests import Session
class MyTestCase(LiveServerTestCase):
    def test_url(self):
        s = Session()
        s.get("{}/api/people'.format(self.server_url))
        s.get("{}/api/cats'.format(self.server_url))

This could look a lot prettier by doing;

from requests import Session
class MyTestCase(LiveServerTestCase):
    def test_url(self):
        s = Session(prefix_url='http://example.com')
        s.get('/api/people')
        s.get('/api/cats')

In the mean time, I was able to achieve this by using a subclass;

class Session(requests.Session):
    """Helpers for performing API queries"""
    def __init__(self, *args, **kwargs):
        self.prefix_url = kwargs.get('prefix_url', None)
        super(Session, self).__init__(*args, **kwargs)

    def request(self, method, url, *args, **kwargs):
        if self.prefix_url:
            url = "{}{}".format(self.prefix_url, url)
        return super(Session, self).request(method, url, *args, **kwargs)

@kennethreitz would such a PR be accepted? Or does anyone know of a better way this could be done?

@sigmavirus24

This comment has been minimized.

Member

sigmavirus24 commented Apr 21, 2015

As we've described in previous issues (#2445, #133) this is not something we feel belongs in requests itself. There are several libraries that do this in differing ways.

The easiest way to implement this yourself would be to use the urljoin function provided by urllib/urlparse/etc. (depending on your version of Python).

@foxx

This comment has been minimized.

foxx commented Apr 21, 2015

I agree that prefix_url is not the most flexible solution, and hooks/subclasses would be better suited, as suggested by @kennethreitz in #133.

However it's not immediately clear from the docs how to do this, therefore I'd like to propose some sort of docs patch to make this clearer. Would such a patch be accepted?

@sigmavirus24

This comment has been minimized.

Member

sigmavirus24 commented Apr 21, 2015

#133 is extremely old in this case (but represents the decision to not include this). Most of the hooks that existed when Kenneth responded to #133 no longer exist. Currently the only supported hook is the response hook which happens after a requests.Response object has been constructed but before it's given to the user. This, is very late in the process though.

@foxx

This comment has been minimized.

foxx commented Apr 21, 2015

Again, I'd argue this is a common use case which genuinely improves code quality, and is at least worthy of a docs patch. However if @kennethreitz votes down on this, then so be it.

@sigmavirus24

This comment has been minimized.

Member

sigmavirus24 commented Apr 21, 2015

and is at least worthy of a docs patch

I'm not sure I understand what needs to be added to the docs. Would you mind expounding on that?

@foxx

This comment has been minimized.

foxx commented Jun 5, 2015

Apologies for the slow response on this. After trying several different approaches, it seems the most flexible and cleanest approach is subclassing.

Here's an example that meets our particular needs of prepending a URL;

from requests import Session
from urlparse import urljoin

class LiveServerSession(Session):
    def __init__(self, prefix_url):
        self.prefix_url = prefix_url
        super(LiveServerSession, self).__init__()

    def request(self, method, url, *args, **kwargs):
        url = urljoin(self.prefix_url, url)
        return super(LiveServerSession, self).request(method, url, *args, **kwargs)

As for documentation, I'm thinking perhaps a "recipes" page of patterns and user contributed subclasses for achieving common goals, which aren't appropriate for inclusion into the core.

Thoughts @kennethreitz / @sigmavirus24 ?

@sigmavirus24

This comment has been minimized.

Member

sigmavirus24 commented Jun 5, 2015

So we have some recipes sprinkled through the documentation, but for the most part we haven't created one single recipes section. And the recipes that are in the docs are either minimal enough to just work or aren't used by anyone and so we haven't received any bug reports about them. The few really important ones have been abstracted into the requests-toolbelt which adds things as necessary.

You can open a PR there with tests to start a discussion about including this. There are a few things we'll probably want to change because I can already foresee some bug reports coming in if we did ship it in the toolbelt.

@foxx

This comment has been minimized.

foxx commented Jun 5, 2015

Ah, I didn't know about requests-toolbelt, I'll take a look and see about submitting a PR.

@jaraco

This comment has been minimized.

Contributor

jaraco commented Jul 28, 2015

@foxx Did you ever end up submitting a PR with this implementation? I wanted this feature today.

@sigmavirus24

This comment has been minimized.

Member

sigmavirus24 commented Jul 28, 2015

@jaraco double checking the toolbelt, no it does not seem as if @foxx ever did.

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