Issue2451
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.
Created on 2008-03-21 23:16 by jjlee, last changed 2022-04-11 14:56 by admin. 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) * | 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) * | 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) * | 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) * | 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) * | Date: 2008-04-25 20:29 | |
Yes! :) |
|||
msg67502 - (view) | Author: Facundo Batista (facundobatista) * | 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) * | Date: 2008-06-07 13:36 | |
Applied the rest of the patch regarding test_urllib2net.py. Thank you! |
History | |||
---|---|---|---|
Date | User | Action | Args |
2022-04-11 14:56:32 | admin | set | github: 46703 |
2008-06-07 13:37:00 | facundobatista | set | status: open -> closed resolution: accepted messages: + msg67806 |
2008-05-29 16:42:29 | facundobatista | set | messages: + msg67502 |
2008-05-26 21:12:41 | gregory.p.smith | set | priority: high nosy: + gregory.p.smith |
2008-04-25 20:29:07 | facundobatista | set | assignee: facundobatista messages: + msg65808 |
2008-04-25 19:42:40 | jjlee | set | messages: + msg65805 |
2008-03-30 18:20:54 | benjamin.peterson | set | nosy:
+ benjamin.peterson messages: + msg64756 |
2008-03-30 16:40:06 | jjlee | set | messages: + msg64752 |
2008-03-30 16:34:25 | jjlee | set | messages: + msg64751 |
2008-03-30 16:28:28 | jjlee | set | files:
+ issue2451.patch keywords: + patch messages: + msg64750 |
2008-03-27 19:37:04 | jjlee | set | messages: + msg64604 |
2008-03-27 12:28:02 | facundobatista | set | status: closed -> open resolution: wont fix -> (no value) messages: + msg64585 |
2008-03-25 22:41:23 | amak | set | nosy:
+ amak messages: + msg64525 |
2008-03-22 00:27:24 | facundobatista | set | messages: + msg64301 |
2008-03-22 00:00:17 | jjlee | set | messages: + msg64300 |
2008-03-21 23:30:34 | facundobatista | set | status: open -> closed resolution: wont fix messages: + msg64297 nosy: + facundobatista |
2008-03-21 23:16:17 | jjlee | create |