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

Document the parameters for urllib3.request() #3177

Merged
merged 9 commits into from
Nov 13, 2023
Merged

Document the parameters for urllib3.request() #3177

merged 9 commits into from
Nov 13, 2023

Conversation

alexwlchan
Copy link
Contributor

@alexwlchan alexwlchan commented Nov 3, 2023

This documents the urllib3.request() function, and a couple of its intermediaries, as described in #3011.

I built the docs locally to check the parameters were appearing correctly in both the user and reference guide. I did get one warning, but I think it was unrelated to my change. (urllib3/src/urllib3/contrib/socks.py:docstring of urllib3.contrib.socks.SOCKSConnection.socket_options:1:py:class reference target not found: connection._TYPE_SOCKET_OPTIONS)

https://urllib3.readthedocs.io/en/stable/reference/urllib3.request.html:

Before After

https://urllib3.readthedocs.io/en/stable/reference/urllib3.poolmanager.html#urllib3.PoolManager.request (linked from request() in the user guide):

Before After

Notes for reviewers

  • Most of the parameter descriptions are copied directly from HTTP(S)ConnectionPool.urlopen(), with the exception of json and fields.
  • I split the patch into individual commits as I was going along because I thought I might end up writing more detailed "where did this text come from" messages, but I can easily squash them down into a single commit if that's preferred.
  • I didn't write a changelog entry because documenting a single function didn't feel like a change that needed one, but I can add one if you think it's important.

Fixes #3011

This is copied from HTTP(S)ConnectionPool.urlopen.
This is copied from HTTP(S)ConnectionPool.urlopen.
This is copied from HTTP(S)ConnectionPool.urlopen.
This is copied from HTTP(S)ConnectionPool.urlopen.  Note that for the
top-level urllib3 method, the omission of the pool headers is deliberate --
this is a high-level convenience method, and the user may not be aware
of the default pool at this point.
These descriptions are copied from HTTP(S)ConnectionPool.urlopen.
@sethmlarson sethmlarson added the Skip Changelog Pull requests that don't require a changelog entry label Nov 6, 2023
Copy link
Member

@illia-v illia-v left a comment

Choose a reason for hiding this comment

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

Thanks!

I was concerned about exposing internal types on the provided screenshots, but the types look good like previously on https://urllib3--3177.org.readthedocs.build/en/3177/reference/urllib3.request.html

Copy link
Member

@sethmlarson sethmlarson left a comment

Choose a reason for hiding this comment

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

LGTM too, thanks @alexwlchan!

@sethmlarson sethmlarson merged commit 4dd4a82 into urllib3:main Nov 13, 2023
20 of 33 checks passed
@sethmlarson
Copy link
Member

@alexwlchan This issue is eligible for a $100 bounty on our Open Collective! You can submit an expense there linking to this pull request and we'll review it :) Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog Pull requests that don't require a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document the parameters for urllib3.request()
3 participants