8000 bpo-12707: deprecate info(), geturl(), getcode() methods in favor of headers, url, and status properties for HTTPResponse and addinfourl by epicfaace · Pull Request #11447 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

bpo-12707: deprecate info(), geturl(), getcode() methods in favor of headers, url, and status properties for HTTPResponse and addinfourl #11447

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.

Sign up for GitHub

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

Merged
merged 31 commits into from
Sep 13, 2019
Merged

bpo-12707: deprecate info(), geturl(), getcode() methods in favor of headers, url, and status properties for HTTPResponse and addinfourl #11447

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
18a7eac
bpo-12707: deprecate info(), geturl(), getcode() from HTTPResponse an…
epicfaace Jan 6, 2019
2e93da4
bpo-12707: rename 'code' attribute to 'status'
epicfaace Jan 6, 2019
2db7e1f
bpo-12707: Document properties and deprecation of methods
epicfaace Jan 6, 2019
beadb34
bpo-21257: remove deprecation warnings
epicfaace Jan 6, 2019
2a16f9b
bpo-21257: fix whitespace
epicfaace Jan 6, 2019
f07f456
bpo-12707: remove deprecation warnings from doc
epicfaace Jan 6, 2019
5640148
bpo-12707: remove deprecation warning for addurlinfo.getstatus() in docs
epicfaace Jan 6, 2019
51cd8ad
bpo-12707: rename code to status in test
epicfaace Jan 7, 2019
c199098
bpo-12707: rename info to headers in tests
epicfaace Jan 7, 2019
58ced25
Apply suggestions from code review
eamanu Jan 7, 2019
13d373f
bpo-12707: Add links to doc
epicfaace Jan 7, 2019
357c8b4
bpo-12707 merge changes
epicfaace Jan 7, 2019
b99fb9f
bpo-12707: remove empty line
epicfaace Jan 7, 2019
1cf719c
bpo-12707: fix typos
epicfaace Jan 9, 2019
2c3a63f
bpo-12707: add versionadded directive
epicfaace Jan 9, 2019
c57eb8c
bpo-12707: recommendation to use addinfourl.status instead of .code
epicfaace Jan 9, 2019
2174ea5
Update Doc/library/urllib.request.rst
merwok Jan 9, 2019
47f1656
bpo-12707: use email.message.EmailMessage
epicfaace Jan 9, 2019
10d4720
bpo-12707: clean up wording for headers
epicfaace Jan 9, 2019
1a36dfc
Merge branch 'bpo-12707' of https://github.com/epicfaace/cpython into…
epicfaace Jan 9, 2019
c0e78bc
bpo-12707: re-add deprecation wording in doc
epicfaace Jan 11, 2019
febc206
bpo-12707: Separate tests for new and old attributes
epicfaace Jan 11, 2019
73a5a9a
Update Lib/urllib/request.py
merwok Jan 15, 2019
1f8b27b
bpo-12707: fix indentation and formatting
epicfaace Jan 15, 2019
ac31a4a
Merge branch 'bpo-12707' of https://github.com/epicfaace/cpython into…
epicfaace Jan 15, 2019
c9e8110
bpo-12707: fix indentation
epicfaace Jan 15, 2019
61385d8
bpo-12707: remove reST from docstring
epicfaace Jan 15, 2019
2ee88dd
📜🤖 Added by blurb_it.
blurb-it[bot] Aug 27, 2019
d7575f0
Make NEWS more clear
epicfaace Sep 13, 2019
628c2f0
add Deprecated warnings and update versionadded
epicfaace Sep 13, 2019
5f5b2f9
Fix the call of the 'deprecated' directive
matrixise Sep 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
bpo-12707: Add links to doc
  • Loading branch information
@epicfaace
epicfaace committed Jan 7, 2019
commit 13d373f3ee4350b83e462b8b21b3968580b072a2
6 changes: 3 additions & 3 deletions Doc/library/http.client.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,15 +487,15 @@ statement.

.. method:: HTTPResponse.geturl()

Returns the URL of the resource retrieved. Recommended to use *HTTPResponse.url* instead.
Returns the URL of the resource retrieved. Recommended to use :attr:`~HTTPResponse.url` instead.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this could be condensed to:

Deprecated method equivalent to :attr:`HTTPResponse.url`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is that okay to call it "deprecated" even when it doesn't return any DeprecationWarnings when used?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, we name that «doc-deprecated», it’s a useful degree of deprecation.
(i.e. don’t break code that is fine, don’t annoy with warnings, just recommend the obvious way to do it in docs for people who want that)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I've went ahead and called it "deprecated" in the documentation. How do you keep track of which things mentioned in the documentation are deprecated and which are doc-deprecated?


.. method:: HTTPResponse.info()

Returns the response headers. Recommended to use *HTTPResponse.headers* instead.
Returns the response headers. Recommended to use :attr:`~HTTPResponse.headers` instead.

.. method:: HTTPResponse.getstatus()

Returns the status. Recommended to use *HTTPResponse.status* instead.
Returns the status. Recommended to use :attr:`~HTTPResponse.status` instead.

Examples
--------
Expand Down
8 changes: 4 additions & 4 deletions Doc/library/urllib.request.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1571,16 +1571,16 @@ The typical response object is a :class:`urllib.response.addinfourl` instance:

.. attribute:: addinfourl.status

Status code returned by server. Recommended to use *addinfourl.status* instead.
Status code returned by server. Recommended to use :attr:`~addinfourl.status` instead.
Copy link
Member

Choose a reason for hiding this comment

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

  • The recommendation phrase :attr:~addinfourl.status links back to itself while building the docs locally. Also below methods geturl, info also link to addinfourl.status but it's documented to return only status code. Am I missing something here?
  • status seems to be a newly added property. Does this need a versionadded directive?

Copy link
Member

Choose a reason for hiding this comment

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

Ha this is recursive! Was probably copied from the getstatus docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, that was a typo. Added a versionadded directive.

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Status code returned by server. Recommended to use :attr:`~addinfourl.status` instead.
Same as *code*.


.. method:: addinfourl.geturl()

Returns the URL of the resource retrieved. Recommended to use *addinfourl.url* instead.
Returns the URL of the resource retrieved. Recommended to use :attr:`~addinfourl.status` instead.
Copy link
Member

Choose a reason for hiding this comment

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

What do you think of:

Suggested change
Returns the URL of the resource retrieved. Recommended to use :attr:`~addinfourl.status` instead.
Deprecated method equivalent to :attr:`~addinfourl.url`.


.. method:: addinfourl.info()

Returns the response headers. Recommended to use *addinfourl.headers* instead.
Returns the response headers. Recommended to use :attr:`~addinfourl.status` instead.
Copy link
Member

Choose a reason for hiding this comment

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

headers not status!


.. method:: addinfourl.getstatus()

Returns the status. Recommended to use *addinfourl.status* instead.
Returns the status. Recommended to use :attr:`~addinfourl.status` instead.
0