[Patches] [ python-Patches-1750931 ] make build_opener raise exception when not passed a handler

[SourceForge.net]

Patches item #1750931, was opened at 2007-07-10 04:14
Message generated for change (Comment added) made by gbrandl
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1750931&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Library (Lib)
Group: Python 2.6
>Status: Closed
Resolution: Rejected
Priority: 5
Private: No
Submitted By: Robert Renaud (silentstrike)
Assigned to: Nobody/Anonymous (nobody)
Summary: make build_opener raise exception when not passed a handler

Initial Comment:
A bug bit me where I had code that looked like

urllib2.build_opener(func_returning_cookie_jar())

instead of

urllib2.build_opener(HTTPCookieProcessor(func_returning_cookie_jar())

the subsequent http request went through just fine, except it didn't send the cookie.  Nothing complained, except me, now.

Throwing the exception deeper in the code made the unittest fail, so I suspect the "silently ignore handlers which don't actually do any handling" behavior might be by design, but it seems terrible to me.

----------------------------------------------------------------------

>Comment By: Georg Brandl (gbrandl)
Date: 2007-07-12 08:05

Message:
Logged In: YES 
user_id=849994
Originator: NO

Committed #1752270.

----------------------------------------------------------------------

Comment By: Robert Renaud (silentstrike)
Date: 2007-07-11 23:52

Message:
Logged In: YES 
user_id=235882
Originator: YES

Your patch looks good to me, I have no objection so long as this error
will be caught instead of failing without complaint.  I am rejecting my
patch in favor of yours.  Thanks for looking into this.

----------------------------------------------------------------------

Comment By: John J Lee (jjlee)
Date: 2007-07-11 23:16

Message:
Logged In: YES 
user_id=261020
Originator: NO

Patch #1752270 seems more idiomatic (TypeError, not ValueError) and mildly
less intrusive (it does not change the return value of
urllib2.OpenerDirector.add_handler()).  Also, your patch adds a backwards
incompatibility which the new one does not: Currently a correct call to
urllib2.build_opener() may return successfully without adding all of its
handler arguments to the returned urllib2.OpenerDirector, since a
urllib2.BaseHandler instance may have no appropriate methods (no
.http_open(), etc. etc.).  Your patch causes build_opener() (but not
.add_handler()) to raise ValueError in that case.  One could argue that it
should raise an exception, but there is still scope for breakage, e.g.
since these attributes may be added at runtime, and the number of added
attributes may happen to be zero (and in fact, urllib2.ProxyHandler falls
into this category).

I think the docs do not need to be updated if my revised patch is applied,
since they already state that urllib2.OpenerDirector.add_handler() accepts
only urllib2.BaseHandler instances (and typically the possibility of a
TypeError being raised is not explicitly documented in stdlib docs).


----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1750931&group_id=5470