Author italip
Recipients facundobatista, italip, jjlee, ncoghlan, orsenthil, r.david.murray, sidnei
Date 2013-07-26.22:10:09
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1374876609.7.0.953211411584.issue4079@psf.upfronthosting.co.za>
In-reply-to
Content
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.
History
Date User Action Args
2013-07-26 22:10:09italipsetrecipients: + italip, facundobatista, jjlee, ncoghlan, orsenthil, sidnei, r.david.murray
2013-07-26 22:10:09italipsetmessageid: <1374876609.7.0.953211411584.issue4079@psf.upfronthosting.co.za>
2013-07-26 22:10:09italiplinkissue4079 messages
2013-07-26 22:10:09italipcreate