classification
Title: No way to disable socket timeouts in httplib, etc.
Type: Stage:
Components: Library (Lib) Versions: Python 2.6
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: facundobatista Nosy List: amak, benjamin.peterson, facundobatista, gregory.p.smith, jjlee
Priority: high Keywords: patch

Created on 2008-03-21 23:16 by jjlee, last changed 2008-06-07 13:37 by facundobatista. This issue is now closed.

Files
File name Uploaded Description Edit
issue2451.patch jjlee, 2008-03-30 16:28
Messages (15)
msg64293 - (view) Author: John J Lee (jjlee) Date: 2008-03-21 23:16
The new timeout support in 2.6 makes use of new function
socket.create_connection().  socket.create_connection() provides no way
to disable timeouts, other than by relying on socket.getdefaulttimeout()
returning None.  This is unfortunate, because it was the purpose of the
new timeout support to allow control of timeouts without reliance on
global state.

setdefaultsocket.create_connection() should always call
sock.settimeout() with the timeout passed to create_connection(), unless
a special non-None value is passed indicating that the global default is
to be used.  Specific modules may then use that special non-None value
where required, to preserve backwards-compatibility.
msg64297 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2008-03-21 23:30
When the semantics of timout in htmllib (and the other libraries) were
discussed, the community approved the following behaviour:

- setdefaulttimeout() was a hack to allow to set the timeout when
nothing else is available.

- Now that you can pass the timeout when creating the connection, you
shouldn't use the default setting.

So, as to enhance practicality and ease to use, when you pass a timeout
value it will set up the timeout in the socket. When you don't pass
anything, or pass None, it will not set anything. And if you set
previously a default value, you can not override it through this
parameter: you shouldn't been setting the default in first place.

Regards,
msg64300 - (view) Author: John J Lee (jjlee) Date: 2008-03-22 00:00
I see this thread:

http://www.gossamer-threads.com/lists/python/dev/552292

But I don't see an explanation of this API decision there that I understand.

*Because* socket.setdefaulttimeout() is a hack for when nothing else is
available, there should be a way to avoid that global state.

You say that "Now that you can pass the timeout when creating the
connection, you shouldn't use the default setting.".  That's true, for
new code, and for code is able to immediately change -- indeed, that
always has been true.  Code exists that makes use of
socket.setdefaulttimeout(), or requires use of it in order to set a
timeout.  Can you explain why this state of affairs makes it necessary
to force this global state on users of httplib?
msg64301 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2008-03-22 00:27
Two or three threads run in parallel at that time for this issue, don't
remember exactly where this was decided.

> *Because* socket.setdefaulttimeout() is a hack for when nothing else is
> available, there should be a way to avoid that global state.

Yes: don't call setdefaulttimeout().


> socket.setdefaulttimeout(), or requires use of it in order to set a
> timeout.  Can you explain why this state of affairs makes it necessary
> to force this global state on users of httplib?

The issue is that to allow caller to override the default state you
should pass it None, but None is for default timeout value in the call,
so you should be passing a specific object to mean "override default
timeout", etc... all this is well explained in that thread.

Lot of suggestions were handled, and the final decision was that all
that behaviours will complicate unnecessarily the semantics here (with
the cost of not being able to override the global default).

I suggest you to raise the discussion again in python-dev if you want
this decided behaviour to change.
msg64525 - (view) Author: Alan Kennedy (amak) * Date: 2008-03-25 22:41
The best solution to this problem was pointed out by Josiah Carlson at
the time; Facundo expressed a preference for Josiah's solution, so it
must have been an oversight that the solution didn't make it into the patch.

http://www.gossamer-threads.com/lists/python/dev/555108?do=post_view_threaded#555108
http://www.gossamer-threads.com/lists/python/dev/555110?do=post_view_threaded#555110

Repeating the proposed solution here

sentinel = object()

def create_connection(address, timeout=sentinel):
...
    if timeout is not sentinel:
        sock.settimeout(timeout)
... 

Perhaps Fecundo could be persuaded to implement this solution?

Alan.
msg64585 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2008-03-27 12:28
Mmm.... it seems that not only overlooked this final agreement, but also
forgot it! Bloody brain, :(

I'll happily review any proposed patch for this. Alan, maybe you can be
persuaded to submit one? <.5 wink>
msg64604 - (view) Author: John J Lee (jjlee) Date: 2008-03-27 19:37
Great.  I'll try to submit a patch this weekend.
msg64750 - (view) Author: John J Lee (jjlee) Date: 2008-03-30 16:28
I've attached a patch.

My patch introduces one minor issue: it's an inconvenience when wrapping
objects if special default values like socket._GLOBAL_DEFAULT_TIMEOUT
are not public.  However, I think it's not worth making that special
value public in this case, because it's not needed by code that does not
support the socket.getdefaulttimeout() global default timeout.


Patch description
-----------------

 * Change the timeout arguments that were introduced with Facundo's 2.6
timeout changes so that they have the same meaning as the argument of
socket.socket.settimeout() .  Specifically, None means "no timeout"
(previously, there was no way to request that), and there is no special
public timeout value meaning "use the global default
socket.getdefaulttimeout() value" (previously, you could pass None to
request that).  This affects socket, urllib2, urllib (only
urllib.ftpwrapper, for urllib2's benefit, urllib public interface and
behaviour is unchanged), httplib, ftplib, telnetlib, poplib, and smtplib.

 * Fix a test bug: test_urllib2net changed global state by calling
urllib2.install_opener(), which interfered with other tests.

 * In tests, where possible, close sockets by calling high-level methods
(e.g. call ftplib.FTP.close(), rather than poking into the object's
internals to .close() the socket directly).

 * Inconsistent defaulting behaviour in telnetlib was introduced with
the timeout changes (r54608).  Made timeout behave like port in terms of
defaulting behaviour.

 * Improve socket.create_connection() documentation.  Some of these
changes need to be propagated to the individual protocol modules that
call this function (e.g. httplib).

  - Document return value

  - Be more PEP 8-compliant ("Connect to...", not "Connects to...")

  - Remove this sentence, since it seems out of place in API
documentation and unlikely to be true: "Especially useful for
higher-level protocols, it is not normally used directly from
application-level code."

  - Reword to remove any doubt that the timeout applies to (almost) all
blocking operations, not just .connect()

  - Rephrase timeout parameter description for better English style.

  - Note that create_connection() is a convenience function (rather than
part of the thin wrapper around the C API).

  - Make the docstring for create_connection() match the official reST
API docs.


Unresolved issues with the new 2.6 timeout functionality
--------------------------------------------------------

 * http://bugs.python.org/issue2452

 * I did not propagate the changes to socket.create_connection() docs to
all the protocol modules (httplib, etc.).  Probably this change should
be committed separately -- I ran out of time.

 * References in the docs to "the global default timeout setting" are
vague.  They should specifically refer to socket.getdefaulttimeout() . 
This should be done in such a way as to also fix the lack of
documentation of the None special value in the protocol modules
documentation (httplib, etc.).  I should have fixed that as part of this
patch, but ran out of time -- sorry!

 * Inconsistency: CacheFTPHandler has per-handler timeout, per-request
timeout is ignored!  Doc/library/urllib2.rst says (in two places):

"""This actually only work for HTTP, HTTPS, FTP and FTPS connections."""

That's not true. (What about CacheFTPHandler, for instance?)  It's also
unclear why it refers to *connections* rather than URL schemes, handler
classes, and the network operations they perform.  (there's also a
little grammatical error here -- s/work/works/)

 * API weirdness: Only some, special, urllib2.Request objects may be
passed to handlers, because the constructor does not create objects with
a .timeout.  Should it really be per-request anyway (I'm not sure)?
msg64751 - (view) Author: John J Lee (jjlee) Date: 2008-03-30 16:34
Should I also have selected "Python 3.0" from the "Versions" list, BTW?
 Don't know what the proper process is ATM...
msg64752 - (view) Author: John J Lee (jjlee) Date: 2008-03-30 16:40
Me:

"""
This should be done in such a way as to also fix the lack of
documentation of the None special value in the protocol modules
documentation (httplib, etc.).  I should have fixed that as part of this
patch, but ran out of time -- sorry!
"""

Erm, possibly the meaning and allowed values of the timeout arguments
would be more naturally documented by pointing to the socket
.settimeout() method docs, actually.
msg64756 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-03-30 18:20
Marking just 2.6 is fine. The fix will be merged into 3.0
msg65805 - (view) Author: John J Lee (jjlee) Date: 2008-04-25 19:42
Facundo, are you going to review this?
msg65808 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2008-04-25 20:29
Yes! :)
msg67502 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2008-05-29 16:42
Commited (part of this) patch on r63788.

A lot of small details weren't commited, in a big change like this, the
best is to minimize the changes.

What I have left from this commit, but plan to do it later is a fix to
test_urllib2net.py (this is why I'm not closing this issue).

Thank you!!
msg67806 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2008-06-07 13:36
Applied the rest of the patch regarding test_urllib2net.py.

Thank you!
History
Date User Action Args
2008-06-07 13:37:00facundobatistasetstatus: open -> closed
resolution: accepted
messages: + msg67806
2008-05-29 16:42:29facundobatistasetmessages: + msg67502
2008-05-26 21:12:41gregory.p.smithsetpriority: high
nosy: + gregory.p.smith
2008-04-25 20:29:07facundobatistasetassignee: facundobatista
messages: + msg65808
2008-04-25 19:42:40jjleesetmessages: + msg65805
2008-03-30 18:20:54benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg64756
2008-03-30 16:40:06jjleesetmessages: + msg64752
2008-03-30 16:34:25jjleesetmessages: + msg64751
2008-03-30 16:28:28jjleesetfiles: + issue2451.patch
keywords: + patch
messages: + msg64750
2008-03-27 19:37:04jjleesetmessages: + msg64604
2008-03-27 12:28:02facundobatistasetstatus: closed -> open
resolution: wont fix -> (no value)
messages: + msg64585
2008-03-25 22:41:23amaksetnosy: + amak
messages: + msg64525
2008-03-22 00:27:24facundobatistasetmessages: + msg64301
2008-03-22 00:00:17jjleesetmessages: + msg64300
2008-03-21 23:30:34facundobatistasetstatus: open -> closed
resolution: wont fix
messages: + msg64297
nosy: + facundobatista
2008-03-21 23:16:17jjleecreate