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

Reply via email to