Documentation examples.

Thanks for an excellent package. I believe the quick start examples should include a timeout value. This is the more common case, rather than the exception. 

In the more advanced documentation it maybe wise to mention that sockets.getdefaulttimeout() is not used when a value is not specified. 


Thanks for the suggestion! However, I disagree.

In many cases, users find it valuable to add a timeout to their calls. This is entirely reasonable. However, the quickstart code is not an example of 'general use', it's intended to be illustrative and educational. We should not obscure the pedagogical point in the pursuit of some form of pure correctness.

There is a [timeout section](http://docs.python-requests.org/en/latest/user/quickstart/#timeouts) in the quickstart docs, which we believe to be sufficient. Do you disagree?

As for mentioning the default socket timeout, I hadn't spotted that. That may actually be a bug. @sigmavirus24, what do you think: should we use the default socket timeout when no value is provided? If so, how do users specify they want no timeout at all? Am I overthinking the whole thing?


@Lukasa I'm surprised that we don't use the default socket timeout. I'm more surprised that the "default" timeout is not automatically applied to a socket. That said, I don't think there's a good (or easy way) to handle adding this at this juncture. We could consider it for 3.0 but if we truly have no timeout set on sockets by default (which I'm prepared to believe), then we can't change it easily and it _should_ be documented for now.

The thing we should determine is whether we want that behaviour at all. The sad thing is that more often than not, not being able to put a timeout on `socket.getaddrinfo` has affected me more than a connection not timing out appropriately. 


I disagree that it is sufficient as timeouts  are only mentioned at the
very bottom of the page.  Users often base their solutions off of examples
and don't read all the documentation, only what they need to get a result.
 While examples should be brief, they should not lead a user to writing
code that has a pretty strong potential to lock up.

The default socket timeout is used by urllib2 when a timeout is not
specified. This further compounds the above problem for people moving from
that library.  To have no timeout specified I can see either None or 0
being passed as the timeout value.  Explicit is better than implicit.

On Tue, Sep 16, 2014 at 9:26 AM, Cory Benfield notifications@github.com
wrote:

> Thanks for the suggestion! However, I disagree.
> 
> In many cases, users find it valuable to add a timeout to their calls.
> This is entirely reasonable. However, the quickstart code is not an example
> of 'general use', it's intended to be illustrative and educational. We
> should not obscure the pedagogical point in the pursuit of some form of
> pure correctness.
> 
> There is a timeout section
> http://docs.python-requests.org/en/latest/user/quickstart/#timeouts in
> the quickstart docs, which we believe to be sufficient. Do you disagree?
> 
> As for mentioning the default socket timeout, I hadn't spotted that. That
> may actually be a bug. @sigmavirus24 https://github.com/sigmavirus24,
> what do you think: should we use the default socket timeout when no value
> is provided? If so, how do users specify they want no timeout at all? Am I
> overthinking the whole thing?
> 
> —
> Reply to this email directly or view it on GitHub
> https://github.com/kennethreitz/requests/issues/2228#issuecomment-55760567
> .


> @Lukasa I'm surprised that we don't use the default socket timeout. I'm more surprised that the "default" timeout is not automatically applied to a socket.

Ordinarily it would be: this is a consequence of the way our library interacts with urllib3. We build a Timeout object with `None, None` as its values and send that down to urllib3. This looks for a sentinel object: if it finds it it'll set the timeout to the default. We aren't sending that sentinel object, we're sending `None, None`, which sets the socket timeout to `None`, which means it never times out. A bit sad really.

> Users often base their solutions off of examples and don't read all the documentation, only what they need to get a result.

Yes, they do. However, this is not a justification for all examples being bulletproof. Our examples don't catch all exceptions, they don't list all possible failure cases, and they don't explain some of the ins-and-outs of weirder parts of the library. I'd argue they shouldn't. That overwhelms beginners by giving them far too many things to focus on at once, which prevents them from taking away the key lesson.

Our position is that documentation should not be optimised for copy-and-paste coding sessions. You can see this by the fact that our quickstart code examples usually aren't code blocks, but representations of an interactive session. They are intended as a guide, not an example of how to build a production-ready requests application.

The correct way to handle this is not to litter our code examples with irrelevant noise to avoid a footgun in production code, but to remove the footgun entirely. Users should not be subject to the TCP timeout (often very long) in the _default_ case, they should be subject to something vaguely reasonable (in this case the default socket timeout).

Note also that the timeouts are not a panacea. The timeouts apply to each connection event, but if `getaddrinfo` returns many many IP addresses it can take a _very_ long time to time them all out. This is not a _hang_, but it's frequently very close to one (and indeed we've seen it reported that way in the past).

> Explicit is better than implicit.

This is a point that is extremely unclear with the Python community. Let me show you something that is unarguably explicit:

``` python
import urllib2
auth_handler = urllib2.HTTPBasicAuthHandler()
auth_handler.add_password(realm='PDQ Application',
                          uri='https://mahler:8092/site-updates.py',
                          user='klem',
                          passwd='kadidd!ehopper')
opener = urllib2.build_opener(auth_handler)
urllib2.install_opener(opener)
urllib2.urlopen('http://www.example.com/login.html')
```

_Nothing_ happens automatically here, everything is stated explicitly. Realm, the URL to apply to, everything. Here's what we do:

``` python
import requests
r = requests.get('http://www.example.com/login.html', auth=('klem', 'kadidd!ehopper')
```

There is so much that is implicit here. The auth type is implicit (Basic), the realm it applies to is not specified (because _no-one cares_, the realm is basically irrelevant on Basic auth), the URL it applies to is inferred by the library (anything on `http://www.example.com/`). But here's the thing: people love our way, and hate urllib2's way.

The Zen of Python is great, but every line of it needs to be followed by asterisks for qualification. It is not the dogma of Python. You cannot wave it at a solution you do not like to force compliance. And for every clause in it that supports your point, I can find plenty that don't:

> Beautiful is better than ugly.
> Simple is better than complex.
> Readability counts.

We have timeouts, and we mention them in their own section in the quickstart guide. Users who care will find it. And if they don't find it, they clearly don't care about errors at all because the "Errors and Exceptions" section is _below_ the timeouts section!

Our documentation cannot accommodate all cases, so we optimise. The quickstart guide is just that: a quickstart guide. We cannot prevent users doing something else with it, but that shouldn't change what _we_ do with it.

**tl;dr**: I think the documentation isn't the problem, the default behaviour is the problem.


@Lukasa hit every point on the head. Moving forward we can repurpose this issue to discuss setting the default socket timeout _or_ we can close this and open a new issue to discuss that.


On Tue, Sep 16, 2014 at 11:26 AM, Cory Benfield notifications@github.com
wrote:

> @Lukasa https://github.com/Lukasa I'm surprised that we don't use the
> default socket timeout. I'm more surprised that the "default" timeout is
> not automatically applied to a socket.
> 
> Ordinarily it would be: this is a consequence of the way our library
> interacts with urllib3. We build a Timeout object with None, None as its
> values and send that down to urllib3. This looks for a sentinel object: if
> it finds it it'll set the timeout to the default. We aren't sending that
> sentinel object, we're sending None, None, which sets the socket timeout
> to None, which means it never times out. A bit sad really.
> 
> Users often base their solutions off of examples and don't read all the
> documentation, only what they need to get a result.
> 
> Yes, they do. However, this is not a justification for all examples being
> bulletproof. Our examples don't catch all exceptions, they don't list all
> possible failure cases, and they don't explain some of the ins-and-outs of
> weirder parts of the library. I'd argue they shouldn't. That overwhelms
> beginners by giving them far too many things to focus on at once, which
> prevents them from taking away the key lesson.
> 
> Our position is that documentation should not be optimised for
> copy-and-paste coding sessions. You can see this by the fact that our
> quickstart code examples usually aren't code blocks, but representations of
> an interactive session. They are intended as a guide, not an example of how
> to build a production-ready requests application.
> 
> The correct way to handle this is not to litter our code examples with
> irrelevant noise to avoid a footgun in production code, but to remove the
> footgun entirely. Users should not be subject to the TCP timeout (often
> very long) in the _default_ case, they should be subject to something
> vaguely reasonable (in this case the default socket timeout).
> 
> I completely agree with this, my suggestion was only for if you decided to
> leave the "footgun" in place.   I would not consider adding the one
> parameter, littering the code with irrelevant noise, but this maybe a
> matter of taste as to where to draw the line and can appreciate your point
> of view.
> 
> Note also that the timeouts are not a panacea. The timeouts apply to each
> connection event, but if getaddrinfo returns many many IP addresses it
> can take a _very_ long time to time them all out. This is not a _hang_,
> but it's frequently very close to one (and indeed we've seen it reported
> that way in the past).
> 
> Thanks for bringing that to my attention, and that is an interesting point.
> 
>  Explicit is better than implicit.
> 
> This is a point that is extremely unclear with the Python community. Let
> me show you something that is unarguably explicit:
> 
> Good point. Although I still believe it is better to be explicit when the
> behavior differs from the expected (using the default socket timeout).
> 
> import urllib2auth_handler = urllib2.HTTPBasicAuthHandler()auth_handler.add_password(realm='PDQ Application',
>                           uri='https://mahler:8092/site-updates.py',
>                           user='klem',
>                           passwd='kadidd!ehopper')opener = urllib2.build_opener(auth_handler)urllib2.install_opener(opener)urllib2.urlopen('http://www.example.com/login.html')
> 
> _Nothing_ happens automatically here, everything is stated explicitly.
> Realm, the URL to apply to, everything. Here's what we do:
> 
> import requestsr = requests.get('http://www.example.com/login.html', auth=('klem', 'kadidd!ehopper')
> 
> There is so much that is implicit here. The auth type is implicit (Basic),
> the realm it applies to is not specified (because _no-one cares_, the
> realm is basically irrelevant on Basic auth), the URL it applies to is
> inferred by the library (anything on http://www.example.com/). But here's
> the thing: people love our way, and hate urllib2's way.
> 
> The Zen of Python is great, but every line of it needs to be followed by
> asterisks for qualification. It is not the dogma of Python. You cannot wave
> it at a solution you do not like to force compliance. And for every clause
> in it that supports your point, I can find plenty that don't:
> 
> Beautiful is better than ugly.
> Simple is better than complex.
> Readability counts.
> 
> We have timeouts, and we mention them in their own section in the
> quickstart guide. Users who care will find it. And if they don't find it,
> they clearly don't care about errors at all because the "Errors and
> Exceptions" section is _below_ the timeouts section!
> 
> Our documentation cannot accommodate all cases, so we optimise. The
> quickstart guide is just that: a quickstart guide. We cannot prevent users
> doing something else with it, but that shouldn't change what _we_ do with
> it.
> 
> _tl;dr_: I think the documentation isn't the problem, the default
> behaviour is the problem.
> 
> AGREED! I just didn't know if you would be amenable to the change your
> proposing and I whole heartily agree with.

Thank you one and all for your attention to this matter.  It is greatly
appreciated

> —
> Reply to this email directly or view it on GitHub
> https://github.com/kennethreitz/requests/issues/2228#issuecomment-55779615
> .


Excellent, I think we're all on the same page! Now we just have to work out whether we can make this change before 3.0 or whether it's likely to break too many people's code. @sigmavirus24, you suggested we'd have to wait until 3.0: what makes you say that?


So the current default for `timeout` is already `None`. It's also unclear how long ago we stopped using `socket.getdefaulttimeout()`. I'm not sure `timeout=0` will be clear enough to everyone either that it disables _all_ timeouts. I guess I'm more concerned with changing he API too dramatically. In the short term we can easily re-enable the usage of `socket.getdefaulttimeout`, I just can't think of a good value to indicate "never set any timeout, default or otherwise"


I feel like `timeout=0` is probably clear enough.

I think we stopped using it when @kevinburke did his really major timeout work, and we didn't spot that urllib3 uses a sentinel object where we use the `None` pattern. Just some crossed wires.


Hi,

sorry if I'm missing the obvious here, I'm rather new to Python (and to requests as well, of course) and I was thinking of picking this up. I noticed that @Lukasa did some changes in July 2015 after the last comment (https://github.com/kennethreitz/requests/issues/commit/e9a1e35f8989691afe99407d9b65a67684aa8bac). After looking at the code I'm not even sure if I'm getting the point here. Is this one still open? What is the desired behavior?


My (naive) view is that there are three cases:

1) The caller never explicitly sets any timeout. In this case requests should use default values and not tamper with any set timeouts. This is not the case right now as requests sends None, requests do not time out.

2) The caller sets a timeout, requests should pass this on. This happens right now and requests time out.

3) The caller doesn't want any timeout and explicitly tell requests to do so (e. g. timeout=0). This currently causes a problem with urllib3 as it ends with an BlockingIOError: [WinError 10035].

Any change on this will change the behavior of requests and I think that this should be avoided for now. My recommendation is to add a description of the current behavior to master and the API change to proposed.

Your input is appreciated.


> 3) The caller doesn't want any timeout and explicitly tell requests to do so (e. g. timeout=0)

`timeout=0` doesn't mean what you want it to mean. In Python setting a timeout of zero is documented as being the same as switching the socket into nonblocking mode, which Requests and urllib3 do not support. We could override the timeout of 0 to do what you're looking for here, but I think that's rather surprising given that we currently are pretty explicit that we set socket-level timeouts.

I'm not entirely sure I agree with 1), either. While I appreciate the thought behind it, this puts Requests at risk of misbehaving due to weird global state that may have been set _very_ far away from the Requests call indeed.

Frankly, I rather wish that the default socket timeout couldn't be set at all, but that instead it had to be set on a per-socket basis.


I comprehend your concern regarding 1), but I do not see a solution to this unless Requests actively overdrives global states and it it impossible to decide when this is actually desirable, or am I overthinking this?

I was not aware of the meaning of `timeout=0`, I agree that my proposal does not fit in here. I think I will pass this one as it is much more complicated for me right now. I still think that a hint about Requests' current behavior (no explicit option set->no timeout) should be added ("When no timeout is set explicitly, requests do not time out.", e. g.).


@Brausepaul I would be delighted to merge a PR that adds a note in the documentation to clarify this.

Hopefully in the longer term we'll get rid of httplib and get to a place where we can set timeouts in a substantially more sensible manner than we currently do.


I think this issue needs some revision. We might close this one and open a new one with a proper description of the underlying issue regarding timeouts.