Issue 1673007: urllib2 requests history + HEAD support (original) (raw)
Created on 2007-03-03 14:01 by koder_ua, last changed 2022-04-11 14:56 by admin. This issue is now closed.
Messages (26)
Author: KDanilov aka koder (koder_ua)
Date: 2007-03-03 14:01
1)Add history off all sent and received headers/requests to addinfourl object. Save redirections history too.
fd = urllib2.urlopen("http://www.python.org/") print fd.history[0].request_line GET / HTTP/1.1 print fd.history[0].sended [('Accept-Encoding', 'identity'), ('Host', 'www.python.org'), ('Connection', 'cl ose'), ('User-Agent', 'Python-urllib/2.6')]
2)Add support for HEAD (and other) requests:
fd = urllib2.urlopen("http://www.python.org/", request_cmd = "HEAD") print len(fd.read()) 0
Please send email here: koder_dot_mail_at_gmail_dot_com
Author: KDanilov aka koder (koder_ua)
Date: 2007-03-07 09:41
Make diff file with svn and delete previos File Added: http_history_and_rtype_svn.diff
Author: Jason Orendorff (jorend)
Date: 2007-04-20 21:45
koder_ua, what's the history useful for? I'm not against it... exactly... it just seems like I would never use it. If I did want a history of HTTP activity, I would need to maintain it myself. I wouldn't want it tied to a particular object--these urlopen() objects, typically you open them, read them, and then throw them away.
I'm iffy on the API for 2) as well, but I can see the appeal.
The thing is... this patch is really, really rough. In one of the tests, an attribute is called "sended_hdrs" in one place and "sended_headers" in another. (Actually it should be called "request_headers". Likewise "recived" and "res" should each be spelled "response".) The whitespace isn't PEP 8. It's also missing a doc patch.
Author: Daniel Diniz (ajaksu2) * 
Date: 2009-02-12 18:13
Patch has tests too, might need updating.
:)
Author: Senthil Kumaran (orsenthil) * 
Date: 2009-12-20 17:59
Having a HEAD request for urllib2 might be a good idea. I shall use this patch to add the functionality.
But, having a history support in the urllib2 module is not a good idea IMO. It is best left to the clients which might use urllib2.
Author: Ezio Melotti (ezio.melotti) *
Date: 2009-12-20 18:01
+1 for HEAD
Author: Éric Araujo (eric.araujo) *
Date: 2009-12-20 18:14
If you know you want an HEAD request, it means you already know it will be an HTTP request, so why not directly use httplib or httplib2 instead of urllib?
Aside: s/sended/sent/
Cheers
Author: Senthil Kumaran (orsenthil) *
Date: 2009-12-22 02:09
Here is a discussion and explanation from the submitter on what is meant by history of requests. http://mail.python.org/pipermail/python-dev/2007-March/072069.html
Author: Mark Lawrence (BreamoreBoy) *
Date: 2010-07-16 23:45
Seems to be +1 for HEAD, -1 for history, keyword says easy so should be feasable for 3.2. :)
Author: David Stanek (dstanek)
Date: 2010-08-04 03:55
I have attached a patch to add support for HEAD, PUT and DELETE methods. The code review is available here: http://codereview.appspot.com/1696061.
I have started working on another patch that validates that the method is properly set. For instance, it doesn't make sense to have a HEAD or DELETE with post data. The problem is that the interface is so wide open that it is hard to catch all possible user errors. A user could call Request.init correctly, but then set req.method to an invalid method. If there is some interest I'll finish up the patch.
Author: (ipatrol)
Date: 2010-09-02 12:03
Can this be somehow implemented as a bugfix patch as well on other versions?
Author: Éric Araujo (eric.araujo) *
Date: 2010-09-02 13:00
Only stable (2.7 and 3.1) and development versions (3.2) get bug fixes.
Author: Antoine Pitrou (pitrou) *
Date: 2010-09-02 13:42
New features can only go in 3.2. From a quick look, the patch looks ok.
Author: Denver Coneybeare (denversc) *
Date: 2011-03-27 19:41
I decided to take a look at this old, forgotten issue and propose an updated patch. I like the submitter's idea that urllib.Request.init() should take a "method" parameter to override the return value of get_method(). I've created and attached a patch (issue1673007_urllib_Request_method_v1.patch) which implements this functionality, adds unit tests, and updates the documentation.
Author: Ezio Melotti (ezio.melotti) *
Date: 2011-10-10 11:20
Attached an updated patch that addresses the comments of Éric in the review and adds an entry to the whatsnew.
Author: Senthil Kumaran (orsenthil) *
Date: 2011-10-10 13:42
Hi Ezio, I had probably overlooked this one. But It's a very interesting one for me. Do you mind if I commit it ? On Oct 10, 2011 7:20 PM, "Ezio Melotti" <report@bugs.python.org> wrote:
Ezio Melotti <ezio.melotti@gmail.com> added the comment:
Attached an updated patch that addresses the comments of Éric in the review and adds an entry to the whatsnew.
assignee: orsenthil -> ezio.melotti keywords: +needs review nosy: -BreamoreBoy stage: patch review -> commit review Added file: http://bugs.python.org/file23366/issue1673007.diff
Python tracker <report@bugs.python.org> <http://bugs.python.org/issue1673007>
Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/senthil%40uthcode.com
Author: Éric Araujo (eric.araujo) *
Date: 2011-10-10 15:31
I made some more comments.
Author: Ezio Melotti (ezio.melotti) *
Date: 2011-10-11 03:56
We have discussed the API a bit on IRC and these are the outcomes:
- should method always have priority or should 'POST' always be used whenever data is passed?
- if the method is e.g. 'GET' and data is passed, should an error be raised?
- should Request.method be private?
- should the value of Request.method be initialized in the init or can it also be None?
- if the value of Request.method is always initialized, should we deprecate get_method?
IMHO
r = Request(url, data, method=foo)
should behave like
class FooRequest(Request): def get_method(self): return foo r = FooRequest(url, data)
when foo is not None and
class FooRequest(Request): pass r = FooRequest(url, data)
when foo is None, so the answers to the 5 questions would be
- method always has the highest priority, data is used to decide between GET and POST when method is None;
- data is simply ignored if the method doesn't use it -- no errors are raised;
- maybe, currently is not (see below);
- method matches the value passed to the constructor, defaulting to None;
- since it's not initialized, get_method shouldn't be deprecated;
This is also what the patch implements.
Regarding 3), one might look at Request.method and see None instead of GET/POST, and this might be confusing. We could document that get_method() should be used instead to see what method will be actually used or make Request.method private and avoid the problem. If we do so there won't be a way to change the method after instance creation (but that wasn't even supported before[0], so it's probably not a problem). OTOH, the other args (e.g. data) are available and documented. I'm fine with either documenting that get_method() should be used instead or making Request.method private.
Another approach is to move the get_method() logic in the init, set self.method to method (if specified) or either POST (if there are data) or GET. Then have get_method() simply return self.method (and possibly deprecate it).
[0]: it's actually possible to replace the get_method() method, but if Request.method is public one could set it to a new value or to None to restore the normal behavior (GET or POST depending on data).
Author: Senthil Kumaran (orsenthil) *
Date: 2011-10-11 05:42
Our discussion stemmed from this point. If you look at the change proposed, Request class is taking a new parameter by name 'method' and it is initialized to None:
class Request:
def __init__(self, url, data=None, headers={},origin_req_host=None, unverifiable=False):
origin_req_host=None, unverifiable=False,method=None):
But that actually defaults to GET method in terms of HTTP request for often used scenarios where the only required parameter (url) is sent.
This happens in the get_method call:
def get_method(self):if self.data is not None:
"""Return a string indicating the HTTP request method."""if self.method is not None:return self.methodelif self.data is not None: return "POST" else: return "GET"
Since, it is understood that the default action of Request(url) is to do a GET, I proposed that we have Request's method parameter default to GET instead of None, so the change would look like:
class Request:
def __init__(self, url, data=None, headers={},origin_req_host=None, unverifiable=False):
origin_req_host=None, unverifiable=False,method="GET"):
And it is more meaningful when someone is looking at the Request signature. Specifying method=None and implicitly meaning it as "GET" for normal situations was not intuitive to me. (This is not case when we do not pass an explicit method arg).
At this point, Ezio's summary of API changes discussed becomes interesting. I read again and it seems to me that, the assumption is get_method is an important method which determines what method should be used and method should be given preference over data.
My point is, get_method is an useful, helper function that is helpful is sending the correct method to the http.client code which does the actual task. In the current situation, get_method "determines" based on data parameter should it send a "GET" or a "POST", but if we start using method=arg then, get_method should just return what was initialized by the method arg (defaulting to "GET").
- The next problem comes when a user has specified both data and method="GET". This becomes an invalid scenario, but a decision has been to taken as what should be given preference?
As the user has sent "data", should the request be considered a POST?
But user specified it as "GET" (intentionally or by mistake), so should the data not be used and Request should do only a GET?
Or should we throw an error?
My personal on this is -1 on throwing an error and when data is sent, just do the POST (data overrides method).
BTW, this needs to discussed irrespective of point 1). But having method="GET" could give raise to his scenario more often. A person would just send data and forget about changing the method to "POST".
Coming to specific questions which Ezio pointed:
My take:
- should method always have priority or should 'POST' always be used whenever data is passed?
If data is passed use POST.
- if the method is e.g. 'GET' and data is passed, should an error be raised?
Nope, give data the priority and do POST. (As urllib is currently doing)
- should Request.method be private?
Not necessarily, it should be public.
- should the value of Request.method be initialized in the init or can it also be None?
My take - It should be initialized to default (GET), instead of None.
- if the value of Request.method is always initialized, should we deprecate get_method?
This is an interesting question. get_method essentially becomes less useful or it could serve as an arbiter when data and GET is sent and may be used as reliable way to get the Request's method. It should not be deprecated.
Author: Patrick Westerhoff (poke)
Date: 2011-10-11 09:01
Senthil, I highly disagree with what you said:
The next problem comes when a user has specified both data and method="GET". This becomes an invalid scenario, but a decision has been to taken as what should be given preference? That is incorrect, RFC2616 states that the server should ignore the message body when the request method does not define any semantics for it, but there is nothing that makes the inclusion of a message body along with the GET request method invalid.
- As the user has sent "data", should the request be considered a POST? No, absolutely not. Sending data via other request methods, like DELETE or PUT, is semantically correct and should be supported completely if we are going to include a way to set the request method. If I set the method to PUT, and include data, I don’t want the library to override that to POST just because I set data.
- But user specified it as "GET" (intentionally or by mistake), so should the data not be used and Request should do only a GET? If I data is included and the request method is explicitely set to GET, make a GET request and include the data in the request body. It might not be a semantically good decision to send data over GET, but it still is not disallowed and as such should be possible (for whatever reasons).
- Or should we throw an error? We especially should’t throw an error, as this is not invalid at all.
A person would just send data and forget about changing the method to "POST". I agree that the library should still default to POST if data is included and the request method was not explicitely set (see also below).
- should method always have priority or should 'POST' always be used whenever data is passed? If data is passed use POST. No, if data is passed and no special method is set, use POST, otherwise use the method the user specified, because that is what he expects.
- if the method is e.g. 'GET' and data is passed, should an error be raised? Nope, give data the priority and do POST. (As urllib is currently doing) Don't give data any priority if the method was set explicitely. Otherwise the ability to set a custom method is totally useless, especially with request methods where a message body is semantically useful (i.e. all other than HEAD and GET).
- should Request.method be private? Not necessarily, it should be public. Depends on the way the method will be set. Looking at the way, the other request parameters are set (especially with the accessors being “deprecated”), it makes sense to leave this public.
- should the value of Request.method be initialized in the init or can it also be None? My take - It should be initialized to default (GET), instead of None. Initializing it to GET will make the implementation difficult, especially if we want to continue supporting the behaviour of setting the method to POST when data is changed (and the method was not explicitely set). Unless we override the built-in property accessors I don’t think this is possible.
- if the value of Request.method is always initialized, should we deprecate get_method? This is an interesting question. get_method essentially becomes less useful or it could serve as an arbiter when data and GET is sent and may be used as reliable way to get the Request's method. It should not be deprecated. To my understanding, and this is also why I provided the same patch on the duplicate bug,
get_methodis used by the other libraries to get the used request method. Unless we change the other libraries to determine the method in a different way, deprecatingget_methodwon’t get us anywhere.
What I tried to respect with the patch, and I think this was also Denver’s intention, is to add this functionality without breaking the current behaviour. That behaviour is that GET is default, and POST is set as default if there is any data. The functionality requires that the request method can be set (and the default behaviour can be overriden) without looking at the data (as explained above).
Ideally I would probably like to see the functionality of get_method done in another library, which performs the request. I.e. check request.method and use that if it’s set, otherwise check request.data and choose either POST or GET. But again this would require far too many changes in other libraries for this simple change.
And again on the data property: I think the name “data” is a bit confusing. Request does not provide any encoding details on that “data”, and it is actually just the request body in its original form. What I usually do in my subclass of Request is to provide a way to encode the data I pass to the constructor (often even with multipart encoding for file streams), while the request.data attribute to me still means “request body”.
Regarding those questions on the implementation, I agree with Ezio, and as I said this is probably the only way to add this functionality without breaking previous usages. And if we break previous usages (or restrict its functionality with weird priority rules based on the data), we better not add this functionality at all.
Author: Senthil Kumaran (orsenthil) *
Date: 2011-10-11 10:02
Patrick,
Lots of valid points. I had not looked at the RFC spec when I mentioned about data over request (GET) method, but was trying to derive the current functionality of module (so that users can have a seamless experience) with additional method="GET" as default. Note, my intention was to be explicit when we give method arg.
But yeah, when user has specified the methods (PUT/DELETE etc) and given the data, correct rules should apply on how that method should deal with data.
As you pointed out to RFC, I realize RFC clearly points out the data (message-body) should be ignored and method should be given preference, whenever specification of method has nothing to with data. I should take back my argument on giving data as preference even over GET.
Now, question arises- Can we in anyway default the method="GET" and maintain compatibility as well consistency with user expectations? At the moment, just by sending data over Request, the method is assumed to be POST. If that is not possible, then the way, current patch does seems to be a good way to acheive the purpose.
Author: Roundup Robot (python-dev)
Date: 2011-10-16 15:55
New changeset 0a0aafaa9bf5 by Senthil Kumaran in branch 'default': Fix closes issue 1673007 urllib.request to support HEAD requests with a new method arg. http://hg.python.org/cpython/rev/0a0aafaa9bf5
Author: Senthil Kumaran (orsenthil) *
Date: 2011-10-16 15:56
I have committed the patch accomodating the doc review comments which Ezio had mentioned. At the moment, the current way seems to be most backwards compatible one and did not want to delay it. Let's hope we get some feedback on this method arg. Thanks!
Author: Éric Araujo (eric.araujo) *
Date: 2011-10-17 13:07
Attached patch changes one occurrence of ugly whitespace, changes “not x == y” to “x != y” and “not x in y” to “x not in y”. Senthil, feel free to apply none, some or all of these minor cleanups.
Author: Senthil Kumaran (orsenthil) *
Date: 2011-10-17 17:19
Hi Eric,
The changes suggested in the patch are good for readability, I shall include them all. Thanks!
Author: Éric Araujo (eric.araujo) *
Date: 2011-10-18 16:42
Doc patch to fix a reST error and tweak a few things.
History
Date
User
Action
Args
2022-04-11 14:56:22
admin
set
github: 44649
2012-02-29 13:09:45
eric.araujo
link
2011-10-18 16:42:25
eric.araujo
set
files: + urllib.request-doc.diff
messages: +
2011-10-17 17:19:40
orsenthil
set
messages: +
2011-10-17 13:07:27
eric.araujo
set
files: + urllib.request-cleanup.diff
messages: +
2011-10-16 15:56:59
orsenthil
set
messages: +
2011-10-16 15:55:00
python-dev
set
status: open -> closed
nosy: + python-dev
messages: +
resolution: fixed
stage: commit review -> resolved
2011-10-11 10:02:08
orsenthil
set
messages: +
2011-10-11 09:01:26
poke
set
messages: +
2011-10-11 05:42:18
orsenthil
set
messages: +
2011-10-11 03:56:52
ezio.melotti
set
messages: +
2011-10-11 02:38:12
ezio.melotti
set
assignee: ezio.melotti -> orsenthil
2011-10-10 15:31:09
eric.araujo
set
messages: +
2011-10-10 13:42:37
orsenthil
set
messages: +
2011-10-10 11:20:41
ezio.melotti
set
files: + issue1673007.diff
assignee: orsenthil -> ezio.melotti
keywords: + needs review
nosy: - BreamoreBoy
messages: +
stage: patch review -> commit review
2011-10-09 22:44:22
poke
set
nosy: + poke
2011-10-09 22:36:59
ezio.melotti
link
2011-03-28 14:47:13
brian.curtin
link
2011-03-28 14:45:43
brian.curtin
set
nosy: + brian.curtin
2011-03-27 21:26:53
santoso.wijaya
set
nosy: + santoso.wijaya
versions: + Python 3.3, - Python 3.2
2011-03-27 19:41:39
denversc
set
files: + issue1673007_urllib_Request_method_v1.patch
nosy: + denversc
messages: +
2010-09-02 13:42:47
pitrou
set
nosy: + pitrou
messages: +
versions: - Python 3.1, Python 2.7
2010-09-02 13:00:09
eric.araujo
set
messages: +
2010-09-02 12:03:47
ipatrol
set
nosy: + ipatrol
messages: +
versions: + Python 3.1, Python 2.7
2010-08-09 17:42:16
jackdied
set
nosy: + jackdied
2010-08-04 03:55:13
dstanek
set
files: + 1673007_methods.patch
nosy: + dstanek
messages: +
2010-07-16 23:45:13
BreamoreBoy
set
nosy: + BreamoreBoy
messages: +
versions: - Python 2.7
2009-12-22 02:09:39
orsenthil
set
messages: +
2009-12-20 18:14:42
eric.araujo
set
nosy: + eric.araujo
messages: +
2009-12-20 18:01:55
ezio.melotti
set
messages: +
versions: + Python 3.2
2009-12-20 17:59:47
orsenthil
set
messages: +
2009-10-19 02:56:48
ezio.melotti
set
nosy: + ezio.melotti
2009-10-17 01:22:16
orsenthil
set
assignee: orsenthil
2009-04-22 17:25:51
ajaksu2
set
keywords: + easy
2009-02-13 01:21:08
ajaksu2
set
nosy: + jjlee
2009-02-12 18:13:41
ajaksu2
set
nosy: + ajaksu2, orsenthil
stage: patch review
type: enhancement
messages: +
versions: + Python 2.7, - Python 2.6
2007-03-03 14:01:08
koder_ua
create