Some proposed changes to allow theAbstractHTTPHandler use the value ofhttp.client.HTTPConnection.debuglevel if nodebuglevel is passed toAbstractHTTPHandler's constructor. This has to be done in the constructor body because argument default values are evaluated at function definition evaluation time andhttp.client.HTTPConnection.debuglevel could be set afterurllib.request is imported.

With these proposed changes,AbstractHTTPHandler andHTTPSHandler now respect both sources ofdebuglevel. If the value is not set in the constructor arguments, the constructor will source the value fromhttp.client.HTTPConnection.debuglevel.

Using the globalhttp.client.HTTPConnection.debuglevel:

wheeler@fedora:~/cpython$ pythonPython 3.10.7 (main, Sep  7 2022, 00:00:00) [GCC 12.2.1 20220819 (Red Hat 12.2.1-1)] on linuxType "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: 486527header: Cache-Control: max-age=604800header: Content-Type: text/html; charset=UTF-8header: Date: Thu, 10 Nov 2022 22:00:09 GMTheader: Etag: "3147526947+gzip+ident"header: Expires: Thu, 17 Nov 2022 22:00:09 GMTheader: Last-Modified: Thu, 17 Oct 2019 07:18:26 GMTheader: Server: ECS (cha/80C2)header: Vary: Accept-Encodingheader: X-Cache: HITheader: Content-Length: 1256header: Connection: close<http.client.HTTPResponse object at 0x7f322c674a00>>>>

Using thedebuglevel constructor parameter:

wheeler@fedora:~/cpython$ pythonPython 3.10.7 (main, Sep  7 2022, 00:00:00) [GCC 12.2.1 20220819 (Red Hat 12.2.1-1)] on linuxType "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: 567191header: Cache-Control: max-age=604800header: Content-Type: text/html; charset=UTF-8header: Date: Thu, 10 Nov 2022 22:06:43 GMTheader: Etag: "3147526947+ident"header: Expires: Thu, 17 Nov 2022 22:06:43 GMTheader: Last-Modified: Thu, 17 Oct 2019 07:18:26 GMTheader: Server: ECS (cha/8096)header: Vary: Accept-Encodingheader: X-Cache: HITheader: Content-Length: 1256header: Connection: close<http.client.HTTPResponse object at 0x7ff801e373d0>>>>

Issue:urllib.request.urlopen() no longer respects thehttp.client.HTTPConnection.debuglevel flag #99352

bedevere-bot mentioned this pull request

Nov 10, 2022

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

Closed

bedevere-bot added the awaiting review label

Nov 10, 2022

Copy link

bedevere-bot commentedNov 10, 2022

Most changes to Pythonrequire a NEWS entry.

Please add it using theblurb_it web app or theblurb command-line tool.

Copy link

ghost commentedNov 10, 2022•
edited by ghost
Loading

All commit authors signed the Contributor License Agreement.

wheelerlaw force-pushed thebugfix/respect-http-connection-debuglevel branch from353519d to518d0e2Compare

November 10, 2022 22:14

Copy link

bedevere-bot commentedNov 10, 2022

Most changes to Pythonrequire a NEWS entry.

Please add it using theblurb_it web app or theblurb command-line tool.

wheelerlaw force-pushed thebugfix/respect-http-connection-debuglevel branch fromac4037b tocc7f8d9Compare

November 10, 2022 22:28

Copy link

ContributorAuthor

wheelerlaw commentedDec 7, 2022

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

hauntsaninja requested a review fromorsenthil

January 20, 2023 04:54

gpshead requested changes

Feb 7, 2023

View reviewed changes

Copy link

Member

gpshead left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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

Lib/test/test_urllib2.py Outdated

		@@ -1048,7 +1057,30 @@ def test_http_body_array(self):
		newreq = h.do_request_(req)
		self.assertEqual(int(newreq.get_header('Content-length')),16)

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

Copy link

Member

gpsheadFeb 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

use awith 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 added awaiting changes and removed awaiting review labels

Feb 7, 2023

Copy link

bedevere-bot commentedFeb 7, 2023

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 phraseI 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.

gpshead self-assigned this

Feb 7, 2023

wheelerlawand others added5 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 thebugfix/respect-http-connection-debuglevel branch from031e8a9 toad096d4Compare

April 21, 2023 01:32

orsenthil requested a review fromgpshead

April 21, 2023 01:32

orsenthil approved these changes

Apr 21, 2023

View reviewed changes

Copy link

Member

orsenthil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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

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 commit5c00a62 intopython: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: ExportHTTPSHandler fromurllib.request#103688

Closed

Copy link

Member

ericsnowcurrently commentedApr 24, 2023

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

Copy link

Member

ericsnowcurrently commentedApr 24, 2023

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

Copy link

Member

orsenthil commentedApr 25, 2023

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

orsenthil mentioned this pull request

Apr 25, 2023

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

Merged

Copy link

Member

orsenthil commentedApr 25, 2023

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

orsenthil added a commit that referenced this pull request

Apr 26, 2023

gh-99352: Ensure HTTPSConnection is available before exercising http…

c986412

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

itamaro pushed a commit to itamaro/cpython that referenced this pull request

Apr 26, 2023

pythongh-99352: Ensure HTTPSConnection is available before exercisin…

2848e41

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

Labels

None yet

5 participants

Movatterモバイル変換

Uh oh!

gh-99352: Respecthttp.client.HTTPConnection.debuglevel inurllib.request.AbstractHTTPHandler#99353

gh-99352: Respecthttp.client.HTTPConnection.debuglevel inurllib.request.AbstractHTTPHandler#99353

Uh oh!

Conversation

wheelerlaw commentedNov 10, 2022• edited by AlexWaygoodLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

bedevere-bot commentedNov 10, 2022

Uh oh!

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

Uh oh!

Uh oh!

bedevere-bot commentedNov 10, 2022

Uh oh!

wheelerlaw commentedDec 7, 2022

Uh oh!

gpshead left a comment

Choose a reason for hiding this comment

Uh oh!

gpsheadFeb 7, 2023

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commentedFeb 7, 2023

Uh oh!

orsenthil left a comment

Choose a reason for hiding this comment

Uh oh!

ericsnowcurrently commentedApr 24, 2023

Uh oh!

ericsnowcurrently commentedApr 24, 2023

Uh oh!

orsenthil commentedApr 25, 2023

Uh oh!

orsenthil commentedApr 25, 2023

Uh oh!

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

wheelerlaw commentedNov 10, 2022•
edited by AlexWaygood
Loading

ghost commentedNov 10, 2022•
edited by ghost
Loading