Skip to content

Type http.client.HTTPConnection.sock - #8386

Merged
srittau merged 3 commits into
python:masterfrom
sobolevn:patch-15
Jul 26, 2022
Merged

Type http.client.HTTPConnection.sock#8386
srittau merged 3 commits into
python:masterfrom
sobolevn:patch-15

Conversation

@github-actions

This comment has been minimized.

@sobolevn

sobolevn commented Jul 25, 2022

Copy link
Copy Markdown
MemberAuthor

Oh, I see. Looks like the idea behind Any here is that different subtypes can use different sock types. What's is the policy in this case?

Do we fallback to Any or do we require # type: ignore[override] for such cases?

@AlexWaygood

AlexWaygood commented Jul 25, 2022

Copy link
Copy Markdown
Member

This looks quite disruptive, and in practice, users can probably be confident that sock will be a socket object after .connect() has been called.

Maybe try sock: socket | Any instead, with a comment explaining that it could be None if connect() has not yet been called?

@github-actions

This comment has been minimized.

@Akuli

Copy link
Copy Markdown
Collaborator

Please add something like sock: SSLSocket | Any into HTTPSConnection. HTTPS connections never use unencrypted socket.sockets.

@sobolevn

Copy link
Copy Markdown
MemberAuthor

@github-actions

Copy link
Copy Markdown
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

urllib3 (https://github.com/urllib3/urllib3)
+ test/with_dummyserver/test_https.py:463: error: Item "socket" of "Union[socket, Any]" has no attribute "server_hostname" [union-attr]+ test/with_dummyserver/test_https.py:784: error: Item "socket" of "Union[socket, Any]" has no attribute "version" [union-attr]+ test/with_dummyserver/test_https.py:882: error: Item "socket" of "Union[socket, Any]" has no attribute "version" [union-attr]

@srittau
srittau merged commit abd893a into python:masterJul 26, 2022
@AlexWaygoodAlexWaygood mentioned this pull request Jul 27, 2022
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@sobolevn@AlexWaygood@Akuli@srittau