Pass method by name, not positionally. (issue 7987046)

4 views
Skip to first unread message

joe.gr...@gmail.com

unread,
Mar 26, 2013, 9:54:20 AM3/26/13
to dhe...@google.com, httpli...@googlegroups.com, re...@codereview-hr.appspotmail.com
Reviewers: dhermes,

Description:
Pass method by name, not positionally.

Fixes issue #252.

Please review this at https://codereview.appspot.com/7987046/

Affected files:
M python2/httplib2/__init__.py
M python3/httplib2/__init__.py
M test/conditional-updates/test.cgi
M test/vary/no-vary.asis


Index: python2/httplib2/__init__.py
===================================================================
--- a/python2/httplib2/__init__.py
+++ b/python2/httplib2/__init__.py
@@ -1380,7 +1380,7 @@
if response.status in [302, 303]:
redirect_method = "GET"
body = None
- (response, content) = self.request(location,
redirect_method, body=body, headers = headers, redirections = redirections
- 1)
+ (response, content) = self.request(location,
method=redirect_method, body=body, headers = headers, redirections =
redirections - 1)
response.previous = old_response
else:
raise RedirectLimit("Redirected more times than
rediection_limit allows.", response, content)
@@ -1522,7 +1522,7 @@
# Should cached permanent redirects be counted in our
redirection count? For now, yes.
if redirections <= 0:
raise RedirectLimit("Redirected more times than
rediection_limit allows.", {}, "")
- (response, new_content) =
self.request(info['-x-permanent-redirect-url'], "GET", headers = headers,
redirections = redirections - 1)
+ (response, new_content) =
self.request(info['-x-permanent-redirect-url'], method="GET", headers =
headers, redirections = redirections - 1)
response.previous = Response(info)
response.previous.fromcache = True
else:
Index: python3/httplib2/__init__.py
===================================================================
--- a/python3/httplib2/__init__.py
+++ b/python3/httplib2/__init__.py
@@ -1089,7 +1089,7 @@
if response.status in [302, 303]:
redirect_method = "GET"
body = None
- (response, content) = self.request(location,
redirect_method, body=body, headers = headers, redirections = redirections
- 1)
+ (response, content) = self.request(location,
method=redirect_method, body=body, headers = headers, redirections =
redirections - 1)
response.previous = old_response
else:
raise RedirectLimit("Redirected more times than
redirection_limit allows.", response, content)
@@ -1224,7 +1224,7 @@
# Should cached permanent redirects be counted in our
redirection count? For now, yes.
if redirections <= 0:
raise RedirectLimit("Redirected more times than
redirection_limit allows.", {}, "")
- (response, new_content) =
self.request(info['-x-permanent-redirect-url'], "GET", headers = headers,
redirections = redirections - 1)
+ (response, new_content) =
self.request(info['-x-permanent-redirect-url'], method="GET", headers =
headers, redirections = redirections - 1)
response.previous = Response(info)
response.previous.fromcache = True
else:
Index: test/conditional-updates/test.cgi
===================================================================
--- a/test/conditional-updates/test.cgi
+++ b/test/conditional-updates/test.cgi
@@ -13,7 +13,7 @@
print "Status: 200 Ok"
print "ETag: 123456789"
print ""
-elif method in ["PUT", "PATCH", "DELETE"]:
+elif method in ["PUT", "DELETE", "PATCH"]:
if "123456789" == os.environ.get('HTTP_IF_MATCH', ''):
print "Status: 200 Ok"
print ""
Index: test/vary/no-vary.asis
===================================================================
--- a/test/vary/no-vary.asis
+++ b/test/vary/no-vary.asis
@@ -1,5 +1,6 @@
#!/usr/bin/tail --lines=+2
Content-Type: text/plain
+Accept-Encoding: identity
Status: 200 OK

We could've been some HTML.


dhe...@google.com

unread,
Mar 26, 2013, 11:38:10 AM3/26/13
to joe.gr...@gmail.com, httpli...@googlegroups.com, re...@codereview-hr.appspotmail.com

https://codereview.appspot.com/7987046/diff/1/python2/httplib2/__init__.py
File python2/httplib2/__init__.py (right):

https://codereview.appspot.com/7987046/diff/1/python2/httplib2/__init__.py#newcode1383
python2/httplib2/__init__.py:1383: (response, content) =
self.request(location, method=redirect_method, body=body, headers =
headers, redirections = redirections - 1)
Can you fix line length and remove the spaces between headers = and
redirections = ?

https://codereview.appspot.com/7987046/diff/1/python2/httplib2/__init__.py#newcode1525
python2/httplib2/__init__.py:1525: (response, new_content) =
self.request(info['-x-permanent-redirect-url'], method="GET", headers =
headers, redirections = redirections - 1)
Can you fix line length, use single quotes around "GET" and remove the
spaces between headers = and redirections = ?

https://codereview.appspot.com/7987046/diff/1/python3/httplib2/__init__.py
File python3/httplib2/__init__.py (right):

https://codereview.appspot.com/7987046/diff/1/python3/httplib2/__init__.py#newcode1092
python3/httplib2/__init__.py:1092: (response, content) =
self.request(location, method=redirect_method, body=body, headers =
headers, redirections = redirections - 1)
Can you fix line length and remove the spaces between headers = and
redirections = ?

https://codereview.appspot.com/7987046/diff/1/python3/httplib2/__init__.py#newcode1227
python3/httplib2/__init__.py:1227: (response, new_content) =
self.request(info['-x-permanent-redirect-url'], method="GET", headers =
headers, redirections = redirections - 1)
Can you fix line length, use single quotes around "GET" and remove the
spaces between headers = and redirections = ?

https://codereview.appspot.com/7987046/diff/1/test/conditional-updates/test.cgi
File test/conditional-updates/test.cgi (right):

https://codereview.appspot.com/7987046/diff/1/test/conditional-updates/test.cgi#newcode16
test/conditional-updates/test.cgi:16: elif method in ["PUT", "DELETE",
"PATCH"]:
What's the reason behind this change?

https://codereview.appspot.com/7987046/diff/1/test/vary/no-vary.asis
File test/vary/no-vary.asis (right):

https://codereview.appspot.com/7987046/diff/1/test/vary/no-vary.asis#newcode3
test/vary/no-vary.asis:3: Accept-Encoding: identity
What's the reason behind this change?

https://codereview.appspot.com/7987046/

joe.gr...@gmail.com

unread,
Mar 26, 2013, 2:07:38 PM3/26/13
to dhe...@google.com, joe.gr...@gmail.com, httpli...@googlegroups.com, re...@codereview-hr.appspotmail.com

https://codereview.appspot.com/7987046/diff/1/python2/httplib2/__init__.py
File python2/httplib2/__init__.py (right):

https://codereview.appspot.com/7987046/diff/1/python2/httplib2/__init__.py#newcode1383
python2/httplib2/__init__.py:1383: (response, content) =
self.request(location, method=redirect_method, body=body, headers =
headers, redirections = redirections - 1)
On 2013/03/26 15:38:10, dhermes wrote:
> Can you fix line length and remove the spaces between headers = and
redirections
> = ?

Done.

https://codereview.appspot.com/7987046/diff/1/python2/httplib2/__init__.py#newcode1525
python2/httplib2/__init__.py:1525: (response, new_content) =
self.request(info['-x-permanent-redirect-url'], method="GET", headers =
headers, redirections = redirections - 1)
On 2013/03/26 15:38:10, dhermes wrote:
> Can you fix line length, use single quotes around "GET" and remove the
spaces
> between headers = and redirections = ?

Done.

https://codereview.appspot.com/7987046/diff/1/python3/httplib2/__init__.py
File python3/httplib2/__init__.py (right):

https://codereview.appspot.com/7987046/diff/1/python3/httplib2/__init__.py#newcode1092
python3/httplib2/__init__.py:1092: (response, content) =
self.request(location, method=redirect_method, body=body, headers =
headers, redirections = redirections - 1)
On 2013/03/26 15:38:10, dhermes wrote:
> Can you fix line length and remove the spaces between headers = and
redirections
> = ?

Done.

https://codereview.appspot.com/7987046/diff/1/python3/httplib2/__init__.py#newcode1227
python3/httplib2/__init__.py:1227: (response, new_content) =
self.request(info['-x-permanent-redirect-url'], method="GET", headers =
headers, redirections = redirections - 1)
On 2013/03/26 15:38:10, dhermes wrote:
> Can you fix line length, use single quotes around "GET" and remove the
spaces
> between headers = and redirections = ?

Done.

https://codereview.appspot.com/7987046/diff/1/test/conditional-updates/test.cgi
File test/conditional-updates/test.cgi (right):

https://codereview.appspot.com/7987046/diff/1/test/conditional-updates/test.cgi#newcode16
test/conditional-updates/test.cgi:16: elif method in ["PUT", "DELETE",
"PATCH"]:
Dropped.

On 2013/03/26 15:38:10, dhermes wrote:
> What's the reason behind this change?

https://codereview.appspot.com/7987046/diff/1/test/vary/no-vary.asis
File test/vary/no-vary.asis (right):

https://codereview.appspot.com/7987046/diff/1/test/vary/no-vary.asis#newcode3
test/vary/no-vary.asis:3: Accept-Encoding: identity
Dropped.

dhe...@google.com

unread,
Mar 26, 2013, 2:11:37 PM3/26/13
to joe.gr...@gmail.com, httpli...@googlegroups.com, re...@codereview-hr.appspotmail.com
LGTM (Still need to fix one of the lint issues)


https://codereview.appspot.com/7987046/diff/8001/python3/httplib2/__init__.py
File python3/httplib2/__init__.py (right):

https://codereview.appspot.com/7987046/diff/8001/python3/httplib2/__init__.py#newcode1231
python3/httplib2/__init__.py:1231: headers = headers, redirections =
redirections - 1)
Still a space between headers = and redirections =

https://codereview.appspot.com/7987046/

joe.gr...@gmail.com

unread,
Mar 26, 2013, 2:15:26 PM3/26/13
to dhe...@google.com, joe.gr...@gmail.com, httpli...@googlegroups.com, re...@codereview-hr.appspotmail.com

https://codereview.appspot.com/7987046/diff/8001/python3/httplib2/__init__.py
File python3/httplib2/__init__.py (right):

https://codereview.appspot.com/7987046/diff/8001/python3/httplib2/__init__.py#newcode1231
python3/httplib2/__init__.py:1231: headers = headers, redirections =
redirections - 1)
On 2013/03/26 18:11:37, dhermes wrote:
> Still a space between headers = and redirections =

Done.

https://codereview.appspot.com/7987046/

joe.gr...@gmail.com

unread,
Mar 26, 2013, 2:19:19 PM3/26/13
to dhe...@google.com, joe.gr...@gmail.com, httpli...@googlegroups.com, re...@codereview-hr.appspotmail.com
Reply all
Reply to author
Forward
0 new messages