gh-145651: Add http.client.{HTTPConnection,HTTPResponse}.__repr__#145653
gh-145651: Add http.client.{HTTPConnection,HTTPResponse}.__repr__#145653JackDanger wants to merge 5 commits intopython:mainfrom
http.client.{HTTPConnection,HTTPResponse}.__repr__#145653Conversation
http.client.{HTTPConnection,HTTPResponse}.__repr__
Lib/http/client.py
Outdated
| self._create_connection = socket.create_connection | ||
|
|
||
| def __repr__(self): | ||
| return '<%s %s:%s>' % (self.__class__.__name__, self.host, self.port if self.port is not None else self.default_port) |
There was a problem hiding this comment.
Please use an if block if self.port is None. You can also use f-strings here as it's new code.
Lib/test/test_httplib.py
Outdated
| if not hasattr(client, 'HTTPSConnection'): | ||
| self.skipTest('ssl support required') |
|
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. |
|
Thanks for making the requested changes! @picnixz: please review the changes made to this pull request. |
| def test_http_response_repr_before_read(self): | ||
| sock = FakeSocket(b'HTTP/1.1 200 OK\r\n\r\n') | ||
| resp = client.HTTPResponse(sock) | ||
| self.assertEqual(repr(resp), '<HTTPResponse>') |
There was a problem hiding this comment.
Would this representation really make sense? it looks a bit... invalid. Maybe <HTTPResponse (notset)> or something like this ("notset" is not the best word though; what does requests and httpx do for such cases?)
| sock = FakeSocket(body) | ||
| resp = client.HTTPResponse(sock) | ||
| resp.begin() | ||
| self.assertEqual(repr(resp), '<HTTPResponse [200 OK]>') |
There was a problem hiding this comment.
Is the representation actually the same as for httpx/requests?
Add informative
__repr__implementations tohttp.client.HTTPConnectionandhttp.client.HTTPResponse.HTTPConnectionshows host and port:<HTTPConnection example.com:80>HTTPResponseshows status and reason:<HTTPResponse [200 OK]>HTTPSConnectioninherits the repr and shows port 443HTTPResponseshows just<HTTPResponse>before headers are readCloses #145651