| msg192773 - (view) |
Author: Barry A. Warsaw (barry) *  |
Date: 2013-07-09 20:11 |
| urllib.request.urlopen() takes a `timeout` argument with a default value, but the default value is "hidden" and undocumented. As implemented, the value is socket._GLOBAL_DEFAULT_TIMEOUT, but even this isn't really correct or useful. The problem comes if you are passing a set of arguments to urlopen() and want to pass in a timeout that is the same as default. Because its undocumented, you have to UTSL to figure out what the value is, and then you have to use a non-public attribute of the socket module. It would be better if urlopen() was documented to default `timeout=None` meaning "use the default timeout". The implementation should then use socket.getdefaulttimeout() when timeout=None. The documentation should also be updated. Now if you want to call urlopen() with the default values, it would just be `urlopen(..., timeout=None, ...)` |
|
|
| msg192774 - (view) |
Author: Barry A. Warsaw (barry) * |
Date: 2013-07-09 20:11 |
| Targeting only 3.4 because, while this should no break any (reasonable ) backward compatibility, it is technically an API change and thus should not get backported. |
|
|
| msg192791 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2013-07-10 05:53 |
| Please see issue 14425 and then let us know if this is still valid. It's been a while since I looked at the code and I no longer remember the details, but I seemed confident in my conclusion at the time... :) |
|
|
| msg192807 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2013-07-10 13:38 |
| See also issue 4079, if you are concerned about timeouts in urllib. |
|
|
| msg192809 - (view) |
Author: Barry A. Warsaw (barry) * |
Date: 2013-07-10 13:54 |
| On Jul 10, 2013, at 05:53 AM, R. David Murray wrote: >Please see issue 14425 and then let us know if this is still valid. It's >been a while since I looked at the code and I no longer remember the details, >but I seemed confident in my conclusion at the time... :) I'm not sure I completely understand the arguments in #14425 and #2451 which it refers to. I could be wrong, but it seems to me that these are arguing against using socket.getdefaulttimeout() in favor of the magic non-public attribute that's currently being used. While I don't like that particular aspect, it's not the real point of *this* bug. Even if socket._GLOBAL_DEFAULT_TIMEOUT is the effective default for `timeout`, I still think using that as the default value for the keyword argument is wrong, both for usability of this API and for the documentation of it. I don't see why `timeout=None` wouldn't be acceptable, but let's assume it isn't. A documented, public sentinel would be acceptable and it could either be defined as: DEFAULT_TIMEOUT = object() or DEFAULT_TIMEOUT = socket._GLOBAL_DEFAULT_TIMEOUT With the latter, the function's signature would then change to: def urlopen(url, data=None, timeout=DEFAULT_TIMEOUT, ...) and nothing functional would change. The documentation update would be much more helpful too. If it weren't for the new module attribute, it could probably even be backported. Then users of this module could do something like this: from urllib.request import DEFAULT_TIMEOUT, urlopen def my_open(url, timeout=None): my_timeout = DEFAULT_TIMEOUT if timeout is None else timeout return urlopen(url, timeout=my_timeout) |
|
|
| msg192812 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2013-07-10 14:16 |
| OK, I reviewed the issue enough to remember: If socket.setdefaulttimeout is never called, then the default timeout is None (no timeout). Since socket.setdefaulttimeout is deprecated, the global default should always be None. Therefore, if you want to use the "default" timeout in a new application, you could pass None as the value for timeout...but of course that probably doesn't do what you want. What you *really* want to do is pass the default timeout value being used by your application, whatever that is, managed however your application/library manages it. If you are writing a library that for some reason wants to retain backward compatibility with setdefaulttimeout (and I think you probably shouldn't be), you could call getdefaulttimeout to get the current default value. Since urlopen is creating a new socket object, this should have the correct semantics. Mind, I don't claim that there can't be a better solution, I'm just trying to explain the current state of affairs as I understand it. However, aside from the ugliness of the current signature for urlopen, at the moment it seems reasonable to me :) That said, this argument is premised firmly on setdefaulttimeout being deprecated, and there is currently no indication of that in its documentation that I can see. |
|
|