Setting session.timeout doesn't do anything

[ikus060]

I was using an old version of requests (I don't know the exact version). When settings the session.timeout it was affecting the timeout value during a call to session.get(). With recent version of requests, this feature doesn't seams to work.

I'm currently using version 2.10.0

Basically, I'm expecting this:

session = requests.session()
session.timeout = 0.0001
session.get('http://localhost/')

To have the same behavior as:

requests.get('http://localhost/', timeout=0.0001)

[Lukasa]

This behaviour was deliberately changed: see #2856, and #2011 which have discussion on them.

[sigmavirus24]

And to be clear, that version must have been prior to 1.0 when this behaviour was removed.

[ikus060]

After reading both issues, I still don't understand why it's not supported. What is the rational behind this decision ?

[ikus060]

Also, whats it the alternative to avoid setting the timeout value for each and every get() request ?

[Lukasa]

@ikus The rationale is that timeouts are not of-a-kind with the things that are set on Sessions. They represent a property of the transport layer and don't semantically apply to a session: how does one "time out" a session? In general they are a per-request property, and defaulting them is a property of the transport layer, which is where Transport Adapters are used to store configuration.

This comment provides an option for defaulting the value.

[ikus060]

I will not argue to much about this decision. But I think it goes against the "Python HTTP Requests for Humans". In order to make sure every GET requests does have a good timeout: (a) I need to review all the code to add timeout as an argument to each call of get() or (b) I need to monkey patch the default HTTPAdapter and provide a default timeout of my own.

Defining the timeout value at the session level would be more human even if the property is not strictly related to the session it self.

[Lukasa]

@ikus060 You do not need to monkeypatch the default HTTPAdapter: just install the correct adapters in at the point where you construct the session.

[ikus060]

If we are talking of this code:

class MyHTTPAdapter(requests.adapters.HTTPAdapter):
    def __init__(self, timeout=None, *args, **kwargs):
        self.timeout = timeout
        super(MyHTTPAdapter, self).__init__(*args, **kwargs)

    def send(self, *args, **kwargs):
        kwargs['timeout'] = self.timeout
        return super(MyHTTPAdapter, self).send(*args, **kwargs)

It look like we are monkeypatching the HTTPAdapter.send() method to provide a default timeout value. At least, would be nice to have an HTTPAdapter constructor accepting a timeout value to avoid defining our own class.

e.g.:

s = requests.Session()
s.mount("http://", requests.adapters.HTTPAdapter(timeout=10))

[Lukasa]

@ikus060 You're not monkeypatching, you're subclassing. This is the entirely expected way to interact with Transport Adapters, and it's done throughout the requests community, as you can see here.

It's not clear to me that a huge amount is gained here by defaulting this. A simpler override, if you're always using the same timeout, is just:

class MyHTTPAdapter(requests.adapters.HTTPAdapter):
    def send(self, *args, **kwargs):
        kwargs['timeout'] = 10
        return super(MyHTTPAdapter, self).send(*args, **kwargs)

At this point we're arguing about saving you 5 LoC. I'm open to having the HTTPAdapter have a timeout default on the object, certainly, but it's not an immediately obvious win.

[nottrobin]

I would totally agree with @ikus060 that:

s = requests.Session()
s.mount("http://", requests.adapters.HTTPAdapter(timeout=10))
s.mount("https://", requests.adapters.HTTPAdapter(timeout=10))

Would be a vastly simpler way for humans to set a timeout, and should be supported. It doesn't require people to understand subclassing and understand the HTTPAdapter.send method to override it.

Since it's easy to support, could it be supported?

Even nicer would be something like:

s = requests.Session(
    transport_adapters={
        ('http://', 'https://'): HTTPAdapter(timeout=10)
    }
)

But I guess that's a longer discussion.

(It would also be nice if you didn't always have to mount both http:// and https://, but that's a different topic).