gh-99352: Respect `http.client.HTTPConnection.debuglevel` in `urllib.request.AbstractHTTPHandler` by wheelerlaw · Pull Request #99353 · python/cpython

wheelerlaw · 2022-11-10T22:10:20Z

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: urllib.request.urlopen() no longer respects the http.client.HTTPConnection.debuglevel flag #99352

bedevere-bot · 2022-11-10T22:10:26Z

Most changes to Python require a NEWS entry.

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

ghost · 2022-11-10T22:10:48Z

All commit authors signed the Contributor License Agreement.

bedevere-bot · 2022-11-10T22:14:51Z

Most changes to Python require a NEWS entry.

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

wheelerlaw · 2022-12-07T01:20:30Z

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

gpshead

(be sure to pull first, i pushed an update to the news entry in the branch)

gpshead · 2023-02-07T19:11:34Z

Lib/test/test_urllib2.py


-    def test_http_handler_debuglevel(self):
+    def test_http_handler_global_debuglevel(self):
+        http.client.HTTPConnection.debuglevel = 1


use a with mock.patch.object(http.client.HTTPConnection, 'debuglevel, 1): style block to set this so that it is undone after the test. same below. I also suggest using a value other than 1 so that it is more obviously correlated with the specific test and not a default from elsewhere.

bedevere-bot · 2023-02-07T19:14:07Z

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.

…ent.HTTPConnection.debuglevel

* Use mock.patch.object instead of settting the module level value. * Used test values to assert the debuglevel.

orsenthil

LGTM. I have addressed the review comments against this change.

ericsnowcurrently · 2023-04-24T23:16:37Z

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

ericsnowcurrently · 2023-04-24T23:17:54Z

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

orsenthil · 2023-04-25T02:06:28Z

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

orsenthil · 2023-04-25T05:29:20Z

Here we go - #103828 ; I can test against the failing buildbot before merging.

…s tests. (#103828) gh-99352: Ensure HTTPSConnection is available before exercising https tests. This will fix the buildbot issue mentioned in #99353

…g https tests. (python#103828) pythongh-99352: Ensure HTTPSConnection is available before exercising https tests. This will fix the buildbot issue mentioned in python#99353

bedevere-bot mentioned this pull request Nov 10, 2022

urllib.request.urlopen() no longer respects the http.client.HTTPConnection.debuglevel flag #99352

Closed

bedevere-bot added the awaiting review label Nov 10, 2022

wheelerlaw force-pushed the bugfix/respect-http-connection-debuglevel branch from 353519d to 518d0e2 Compare November 10, 2022 22:14

wheelerlaw force-pushed the bugfix/respect-http-connection-debuglevel branch from ac4037b to cc7f8d9 Compare November 10, 2022 22:28

hauntsaninja requested a review from orsenthil January 20, 2023 04:54

gpshead requested changes Feb 7, 2023

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting review labels Feb 7, 2023

gpshead self-assigned this Feb 7, 2023

wheelerlaw and others added 5 commits April 20, 2023 18:32

bugfix: let the HTTP- and HTTPSHandlers respect the value of http.cli…

49b00da

…ent.HTTPConnection.debuglevel

add tests

510cf5d

add news

c6f5ee4

ReSTify NEWS and reword a bit.

450b8f2

Address Review Comments.

ad096d4

* Use mock.patch.object instead of settting the module level value. * Used test values to assert the debuglevel.

orsenthil force-pushed the bugfix/respect-http-connection-debuglevel branch from 031e8a9 to ad096d4 Compare April 21, 2023 01:32

orsenthil requested a review from gpshead April 21, 2023 01:32

orsenthil approved these changes Apr 21, 2023

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting changes labels Apr 21, 2023

orsenthil added 🤖 automerge and removed awaiting merge labels Apr 21, 2023

orsenthil merged commit 5c00a62 into python:main Apr 21, 2023

wheelerlaw deleted the bugfix/respect-http-connection-debuglevel branch April 22, 2023 02:47

arhadthedev mentioned this pull request Apr 22, 2023

gh-99352: Export HTTPSHandler from urllib.request #103688

Closed

orsenthil mentioned this pull request Apr 25, 2023

gh-99352: Ensure HTTPSConnection is available before exercising https tests. #103828

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

gh-99352: Respect `http.client.HTTPConnection.debuglevel` in `urllib.request.AbstractHTTPHandler` #99353
orsenthil merged 5 commits intopython:mainfrom
wheelerlaw:bugfix/respect-http-connection-debuglevel

wheelerlaw commented Nov 10, 2022 •

edited by AlexWaygood

Loading

Uh oh!

bedevere-bot commented Nov 10, 2022

Uh oh!

ghost commented Nov 10, 2022 •

edited by ghost

Loading

Uh oh!

bedevere-bot commented Nov 10, 2022

Uh oh!

wheelerlaw commented Dec 7, 2022

Uh oh!

gpshead left a comment

Uh oh!

gpshead Feb 7, 2023

Uh oh!

bedevere-bot commented Feb 7, 2023

Uh oh!

orsenthil left a comment

Uh oh!

ericsnowcurrently commented Apr 24, 2023

Uh oh!

ericsnowcurrently commented Apr 24, 2023

Uh oh!

orsenthil commented Apr 25, 2023

Uh oh!

orsenthil commented Apr 25, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

wheelerlaw commented Nov 10, 2022 • edited by AlexWaygood Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-bot commented Nov 10, 2022

Uh oh!

ghost commented Nov 10, 2022 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-bot commented Nov 10, 2022

Uh oh!

wheelerlaw commented Dec 7, 2022

Uh oh!

gpshead left a comment

Choose a reason for hiding this comment

Uh oh!

gpshead Feb 7, 2023

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Feb 7, 2023

Uh oh!

orsenthil left a comment

Choose a reason for hiding this comment

Uh oh!

ericsnowcurrently commented Apr 24, 2023

Uh oh!

ericsnowcurrently commented Apr 24, 2023

Uh oh!

orsenthil commented Apr 25, 2023

Uh oh!

orsenthil commented Apr 25, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

wheelerlaw commented Nov 10, 2022 •

edited by AlexWaygood

Loading

ghost commented Nov 10, 2022 •

edited by ghost

Loading