Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add stubs for HTTP Handler classes in py2/urllib2 & py3/urllib.request #2710

Merged
merged 3 commits into from Feb 11, 2019

Conversation

Projects
None yet
2 participants
@michaeljb
Copy link
Contributor

commented Dec 21, 2018

Add full annotations for the following classes:

  • Python 2:

    • urllib2.AbstractHTTPHandler
    • urllib2.HTTPHandler
    • urllib2.HTTPsHandler
  • Python 3:

    • urllib.request.AbstractHTTPHandler
    • urllib.request.HTTPHandler
    • urllib.request.HTTPsHandler

This information is largely undocumented, and was obtained by directly examining
the Python stdlib source code:

urllib2.AbstractHTTPHandler.do_open takes as a parameter either
HTTPConnection or HTTPSConnection--one of the classes, not an instance of
either--and constructs an object using only a few of the parameters that either
constructor could use. HTTPConnectionProtocol in stdlib/2/httplib.pyi
follows a similar patten to HTTPConnectionProtocol added to
stdlib/3/http/client.pyi in pull request #2582 to describe the type of the
http_class that is passed to do_open.


We at @nsidc have a Python 3 project where we subclass urllib.request.HTTPSHandler to handle authentication with a particular host. Our subclass implements its own https_request method, which makes some headers changes and then calls the parent method, urllib.request.HTTPSHandler.https_request. When I added mypy to the project, this was one of the errors it produced:

error: "Type[HTTPSHandler]" has no attribute "https_request"

The code had been working before I attempted to add mypy to the project, so I knew the method existed. Investigating why mypy didn't seem to know about that one method lead to this PR.

Pull request #2582 (merged) is somewhat related, and pull request #2688 (open) has a little bit of overlap with the stubs I've changed in urllib2.pyi.


Properties and methods whose names begin with _ are not stubbed here; everything else is.

AbstractHTTPHandler is not documented at all at docs.python.org, so the class has a single "undocumented" comment, rather than each method having the comment.

HTTPHandler and HTTPSHandler have limited documentation:

HTTP Handler class annotations for py2/urllib2 & py3/urllib.request
Add full annotations for the following classes:

* Python 2:

    * `urllib2.AbstractHTTPHandler`
    * `urllib2.HTTPHandler`
    * `urllib2.HTTPsHandler`

* Python 3:

    * `urllib.request.AbstractHTTPHandler`
    * `urllib.request.HTTPHandler`
    * `urllib.request.HTTPsHandler`

This information is largely undocumented, and was obtained by directly examining
the Python source code:

* Python 2 (v2.7.15) - https://github.com/python/cpython/blob/v2.7.15/Lib/urllib2.py#L1115-L1243
* Python 3 (v3.7.1) - https://github.com/python/cpython/blob/v3.7.1/Lib/urllib/request.py#L1224-L1364

`urllib2.AbstractHTTPHandler.do_open` takes as a parameter either
`HTTPConnection` or `HTTPSConnection`--one of the classes, not an instance of
either--and constructs an object using only a few of the parameters that either
constructor could use. `HTTPConnectionProtocol` in `stdlib/2/httplib.pyi`
follows a similar patten to `HTTPConnectionProtocol` added to
`stdlib/3/http/client.pyi` in pull request #2582 to describe the type of the
`http_class` that is passed to `do_open`.
@michaeljb

This comment has been minimized.

Copy link
Contributor Author

commented Jan 7, 2019

Has anyone had a chance to review this yet? I submitted it while people were probably busy with the holidays, but I'd love to see some movement on it this week 😃

@michaeljb michaeljb changed the title HTTP Handler class annotations for py2/urllib2 & py3/urllib.request Add stubs for HTTP Handler classes in py2/urllib2 & py3/urllib.request Jan 7, 2019

@srittau

This comment has been minimized.

Copy link
Collaborator

commented Jan 8, 2019

I have it on my saved notifications list. I hope to be able to review it this week, if no one else is faster.

@srittau
Copy link
Collaborator

left a comment

Sorry for the late review. Life is busy lately.

Show resolved Hide resolved stdlib/2/httplib.pyi Outdated
Show resolved Hide resolved stdlib/2/urllib2.pyi Outdated
Show resolved Hide resolved stdlib/2/urllib2.pyi Outdated
Show resolved Hide resolved stdlib/2/urllib2.pyi Outdated
Show resolved Hide resolved stdlib/3/urllib/request.pyi Outdated
@michaeljb

This comment has been minimized.

Copy link
Contributor Author

commented Feb 11, 2019

Sorry for the late review. Life is busy lately.

And sorry for the late follow-up to the review...same reason basically 😅

@srittau srittau merged commit 1442cc0 into python:master Feb 11, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.