cpython icon indicating copy to clipboard operation
cpython copied to clipboard

gh-99352: Respect `http.client.HTTPConnection.debuglevel` in `urllib.request.AbstractHTTPHandler`

Open wheelerlaw opened this issue 3 years ago • 3 comments
trafficstars

Fixes #99352.

Some proposed changes to allow the AbstractHTTPHandler use the value of http.client.HTTPConnection.debuglevel if no debuglevel is passed to AbstractHTTPHandler's constructor. This has to be done in the constructor body because argument default values are evaluated at function definition evaluation time and http.client.HTTPConnection.debuglevel could be set after urllib.request is imported.

With these proposed changes, AbstractHTTPHandler and HTTPSHandler now respect both sources of debuglevel. If the value is not set in the constructor arguments, the constructor will source the value from http.client.HTTPConnection.debuglevel.

Using the global http.client.HTTPConnection.debuglevel:

wheeler@fedora:~/cpython$ python
Python 3.10.7 (main, Sep  7 2022, 00:00:00) [GCC 12.2.1 20220819 (Red Hat 12.2.1-1)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import http.client
>>> import urllib.request
>>> http.client.HTTPConnection.debuglevel=1
>>> urllib.request.urlopen("http://example.com")
send: b'GET / HTTP/1.1\r\nAccept-Encoding: identity\r\nHost: example.com\r\nUser-Agent: Python-urllib/3.10\r\nConnection: close\r\n\r\n'
reply: 'HTTP/1.1 200 OK\r\n'
header: Age: 486527
header: Cache-Control: max-age=604800
header: Content-Type: text/html; charset=UTF-8
header: Date: Thu, 10 Nov 2022 22:00:09 GMT
header: Etag: "3147526947+gzip+ident"
header: Expires: Thu, 17 Nov 2022 22:00:09 GMT
header: Last-Modified: Thu, 17 Oct 2019 07:18:26 GMT
header: Server: ECS (cha/80C2)
header: Vary: Accept-Encoding
header: X-Cache: HIT
header: Content-Length: 1256
header: Connection: close
<http.client.HTTPResponse object at 0x7f322c674a00>
>>> 

Using the debuglevel constructor parameter:

wheeler@fedora:~/cpython$ python
Python 3.10.7 (main, Sep  7 2022, 00:00:00) [GCC 12.2.1 20220819 (Red Hat 12.2.1-1)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import urllib.request
>>> handler = urllib.request.HTTPHandler(debuglevel=1)
>>> opener = urllib.request.build_opener(handler)
>>> opener.open("http://example.com")
send: b'GET / HTTP/1.1\r\nAccept-Encoding: identity\r\nHost: example.com\r\nUser-Agent: Python-urllib/3.10\r\nConnection: close\r\n\r\n'
reply: 'HTTP/1.1 200 OK\r\n'
header: Age: 567191
header: Cache-Control: max-age=604800
header: Content-Type: text/html; charset=UTF-8
header: Date: Thu, 10 Nov 2022 22:06:43 GMT
header: Etag: "3147526947+ident"
header: Expires: Thu, 17 Nov 2022 22:06:43 GMT
header: Last-Modified: Thu, 17 Oct 2019 07:18:26 GMT
header: Server: ECS (cha/8096)
header: Vary: Accept-Encoding
header: X-Cache: HIT
header: Content-Length: 1256
header: Connection: close
<http.client.HTTPResponse object at 0x7ff801e373d0>
>>> 
  • Issue: gh-99352

wheelerlaw avatar Nov 10 '22 22:11 wheelerlaw

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

bedevere-bot avatar Nov 10 '22 22:11 bedevere-bot

All commit authors signed the Contributor License Agreement.
CLA signed

cpython-cla-bot[bot] avatar Nov 10 '22 22:11 cpython-cla-bot[bot]

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

bedevere-bot avatar Nov 10 '22 22:11 bedevere-bot

Who do I need to mention to get a review on this?

wheelerlaw avatar Dec 07 '22 01:12 wheelerlaw

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

bedevere-bot avatar Feb 07 '23 19:02 bedevere-bot

This appears to have caused failures on several buildbots. For example, https://buildbot.python.org/all/#/builders/15/builds/4336.

ericsnowcurrently avatar Apr 24 '23 23:04 ericsnowcurrently

@orsenthil, this is the one I was talking about.

ericsnowcurrently avatar Apr 24 '23 23:04 ericsnowcurrently

Oh. I see the issue now. Didn't realize we were disabling ssl in some builds. I will fix it. Sorry.

orsenthil avatar Apr 25 '23 02:04 orsenthil

Here we go - https://github.com/python/cpython/pull/103828 ; I can test against the failing buildbot before merging.

orsenthil avatar Apr 25 '23 05:04 orsenthil