This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Adding timeout to socket.py and httplib.py
Type: Stage:
Components: Library (Lib) Versions: Python 2.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: georg.brandl Nosy List: facundobatista, georg.brandl, gvanrossum
Priority: normal Keywords: patch

Created on 2007-03-08 21:32 by facundobatista, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
timeout.diff facundobatista, 2007-03-23 17:51 Patch to code, docs, and test cases
timeout.diff georg.brandl, 2007-03-23 18:33 reviewed version
Messages (11)
msg52125 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2007-03-08 21:32
This is a subset of patch #723312. Like it, adds a NetworkConnection object to socket.py, and then use it from other modules. 

This NetworkConnection basically does what all the other modules do once and again, so no mistery about it (basically calls getaddrinfo over the received address, and try to open a socket to that address).

In this patch I only use this new NetworkConnection from httplib, whose HTTPConnection class now optionally accepts a timeout.

Beyond the changes in socket.py and httplib.py, this patch also includes changes in the docs and new test cases.
msg52126 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-03-12 21:36
I like the idea of refactoring refactoring of the connect() loop, however, I wonder if a class isn't overkill.  The constructor could just store the timeout parameter instead of the networkconnection object, and the connect() method could just call the refactored connect_with_timeout function passing it the host/port pair and the timeout.  But perhaps there's a use case I have missed that appears in one of the classes you haven't patched yet but looked at?
msg52127 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2007-03-13 16:11
In all the other libraries we can store the timeout, and then make a single call to socket.py. 

There's no "refactored connect_with_timeout" in socket.py. The functionality that will be present is one that is repeated several times in the higher level modules. So, there's no need of a "..._with_timeout" name. 

I'll work in a function, in socket.py, called "connect", with this functionality, and the following signature:

  def connect(address, timeout=None):   # being address == (host, port)
     ...
msg52128 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2007-03-15 18:31
I refactored the patch, now it uses function connect() in socket.py as specified in my last comment here.

Of course, tests and docs are also updated in the patch.
File Added: timeout.diff
msg52129 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-03-15 18:53
Georg, can you review this?
msg52130 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2007-03-21 16:24
I updated the patch, reflecting discussion on python-dev:

- The function name changed, now it's _create_connection(). Its signature also changed: now, timeout is mandatorily named.

- HTTPConnection has the posibility to call timeout with a number, and also with None. In both cases, it update sock.timeout (changing effectively the default timeout).

Docs and tests cases are also updated (note: now there's no doc in libsocket.tex, as this function is now private).

File Added: timeout.diff
msg52131 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-03-21 17:12
I may have missed some of the discussion, but I don't understand why it is so important to differentiate between explicit timeout=None and omitting timeout altogether.  I would favor a version where the timeout defaults to None, is a simple positional argument to _create_connection(), and the settimeout() call is skipped when timeout is None.

I also favor making create_connection() public again since it may be useful for user code or 3rd party libraries that implements some protocol.  After all the reality is probably that people have copied the exact loop that's found in httplib and 5 other places into their code anyway, if they care at all about IPv6 support.
msg52132 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2007-03-23 17:51
The patch is updated, reflecting discussion in python-dev and last comment here.

- Just put a timeout default to None. If None, skip settimeout() call.

- Copy the exceptions behaviour that we have actually in the higher
  level libraries, to be sure we aren't breaking anything.

- The function is now public, named "create_connection".

Tests and docs are updated too.
File Added: timeout.diff
msg52133 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-03-23 18:21
Can you check this in yourself?
msg52134 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2007-03-23 18:33
I'm attaching a reviewed version. Basically only nits and one added XXX comment about IPv6.
File Added: timeout.diff
msg52135 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2007-03-23 19:33
I already commited the previous version, :s.

I'm closing this patch, feel free to reopen it if you want me to add the XXX comment.

Regards,
History
Date User Action Args
2022-04-11 14:56:23adminsetgithub: 44685
2007-03-08 21:32:11facundobatistacreate