[issue4079] new urllib2.Request 'timeout' attribute needs to have a default
R. David Murray added the comment: Yes, this is a very good point. (And your passing test is worthwhile, you are correct.) People are expected to be able to write handlers, so clearly the timeout API needs to be documented, if for no other reason than to keep a handler writer from stomping on the timeout attribute inappropriately. But there is a code smell to this API, and I wonder if it is worth someone taking time to think it all through again to see if there is a better way to do it :( -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4079 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4079] new urllib2.Request 'timeout' attribute needs to have a default
Indra Talip added the comment: Ah sorry you are right the tests didn't test the specific case of someone overriding the OpenerDirector. The intent was to demonstrate that adding a default timeout to Request didn't break things. I think you are right about how much of an issue this is. I agree that if you are overriding open you probably should be replicating it's functionality and doing something sane about the timeout parameter. Reading the code and the online docs I think that the more pertinent issue here is that it isn't documented how the timeout is passed to the handlers. That is if someone implements a new handler that could timeout just from reading the documentation it isn't clear how the timeout is passed to the handler. Given that open adds the timeout to the Request object perhaps the issue should be that the timeout attribute should be added to the public api of the Request and the docs modified to suit, thus making it explicit how handlers get the expected timeout. That does uglify the api though as you would have two places where you could set the timeout (on the Request and via open) and currently calling open with a Request, with a non-default timeout, means that open would override the timeout on the Request with whatever was set on the call to open. So overall I'm fairly ambivalent about how much of an issue the original report was. I think the larger issue is that how timeouts are handled/passed to handlers/processors should probably be documented. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4079 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4079] new urllib2.Request 'timeout' attribute needs to have a default
R. David Murray added the comment: Thanks, but the new test doesn't fail. With your test patch, test_default_values fails, but that is explicitly checking for the 'timeout' attribute, which isn't actually part of the public API. The new test passes even without the fix. Reading the original issue, it seems like the problem is a bit more subtle. To quote that issue: * API weirdness: Only some, special, urllib2.Request objects may be passed to handlers, because the constructor does not create objects with a .timeout. The original report here says the problem occurs if a subclass overrides 'open' (like Mechanize does). So that problem will only occur if a request gets passed to a handler without having passed through 'open'. But there is a question whether there is a way for the code that assumes the timeout attribute exists to fail that doesn't involve an override of 'open'. The answer may be no, but I haven't traced the logic or reviewed the API, so I don't know. However, it is clear that if open is overridden, and the code doing so doesn't care about timeouts and so doesn't do the attribute set itself, subsequent code will sometimes fail because it assumes that the attribute does exist. We could write a test that does that kind of override on 'open', and causes one of those code paths that assumes timeout exists to be called. But...I have some question, now, just how real an issue this is...if you override open, you really should be either calling it via super or replicating its functionality...so if there is not in fact any way to cause the lack of a timeout attribute to cause a failure without overriding open, there may not be a real issue here. Presumably mechanize ran into it because a keyword argument (new feature) was added. So the mechanize bug was really that there was a backward compatibility issue between 2.6 and previous versions. Is that still an issue for 2.7? Perhaps: there could still be code someone will forward port to 2.7 that overrides open and doesn't know about timeout. So it is worth fixing, since the fix is simple. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4079 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4079] new urllib2.Request 'timeout' attribute needs to have a default
Indra Talip added the comment: patch adds regressions tests to ensure timeout value from OpenerDirector is honoured and a failing test for a default value for Request.timeout that passes when the patch that initialises Request.timeout is applied. -- Added file: http://bugs.python.org/file31003/issue-4079-tests-1.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4079 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4079] new urllib2.Request 'timeout' attribute needs to have a default
Changes by Indra Talip indra.ta...@gmail.com: -- nosy: +ncoghlan ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4079 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4079] new urllib2.Request 'timeout' attribute needs to have a default
R. David Murray added the comment: Now we need a test :) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4079 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4079] new urllib2.Request 'timeout' attribute needs to have a default
Changes by R. David Murray rdmur...@bitdance.com: -- stage: test needed - patch review status: languishing - open ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4079 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4079] new urllib2.Request 'timeout' attribute needs to have a default
Indra Talip added the comment: patch initializes the 'timeout' attribute on urllib2.Request.__init__() -- keywords: +patch nosy: +italip versions: +Python 3.4 -Python 2.6 Added file: http://bugs.python.org/file30873/issue-4079-1.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4079 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4079] new urllib2.Request 'timeout' attribute needs to have a default
Facundo Batista facu...@taniquetil.com.ar added the comment: I'm ok with the proposed changes from Sidnei (yes, a patch is needed). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4079 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4079] new urllib2.Request 'timeout' attribute needs to have a default
R. David Murray rdmur...@bitdance.com added the comment: Senthil, Facundo, is there a reason this bug shouldn't be fixed, or are we just waiting for someone to come up with a patch? I'm assuming the latter and setting it to languishing, but maybe this will wake somebody up who is interested in fixing it. -- keywords: +easy nosy: +r.david.murray priority: - normal stage: - unit test needed status: open - languishing ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue4079 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4079] new urllib2.Request 'timeout' attribute needs to have a default
John J Lee [EMAIL PROTECTED] added the comment: This bug was known before the release -- unfortunately the original author of the patch didn't fix them in time. This and some other unresolved issues listed are listed on #2451. Somebody should raise bugs about the rest of them too, and fix them. (Senthil, is that the bug you meant to refer to? You pasted this bug's own URL here, presumably you meant to paste the URL for a different bug?) -- nosy: +jjlee ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue4079 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4079] new urllib2.Request 'timeout' attribute needs to have a default
Changes by John J Lee [EMAIL PROTECTED]: -- nosy: +facundobatista ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue4079 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4079] new urllib2.Request 'timeout' attribute needs to have a default
Senthil [EMAIL PROTECTED] added the comment: http://bugs.python.org/issue4079 -- nosy: +orsenthil ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue4079 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue4079] new urllib2.Request 'timeout' attribute needs to have a default
New submission from Sidnei da Silva [EMAIL PROTECTED]: 'urllib2' has introduced a configurable 'timeout' setting by assigning to the 'timeout' attribute of the urllib2.Request object. However the implementation is flawed: - the 'timeout' attribute is set in OpenerDirector.open() and nowhere else - if someone overrides OpenerDirector.open() (btw: mechanize does this), then the 'timeout' attribute will never be set, breaking other parts of the code which require the 'timeout' attribute to be present. A simple workaround for this would be to do one or more of: a) define the 'timeout' attribute as socket._GLOBAL_DEFAULT_TIMEOUT at class-level b) initialize the 'timeout' attribute on urllib2.Request.__init__() -- components: Library (Lib) messages: 74541 nosy: sidnei severity: normal status: open title: new urllib2.Request 'timeout' attribute needs to have a default type: behavior versions: Python 2.6 ___ Python tracker [EMAIL PROTECTED] http://bugs.python.org/issue4079 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
