10000 docs: add an explicit `close()` call to `urlopen` examples without `with` · Issue #130132 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

docs: add an explicit close() call to urlopen examples without with #130132

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

Jump to bottom
Closed
opk12 opened this issue Feb 14, 2025 · 7 comments · Fixed by #130280
Closed

docs: add an explicit close() call to urlopen examples without with #130132

opk12 opened this issue Feb 14, 2025 · 7 comments · Fixed by #130280
Labels
docs Documentation in the Doc dir

Comments

@opk12
Copy link
opk12 commented Feb 14, 2025 •
edited by bedevere-app bot
Loading

Documentation

The documentation for urllib.request uses urlopen() 3 times without either with or manual closing. Although technically possible, this is not recommended.

It is also possible to achieve the same result without using the context manager approach.

>>> import urllib.request
>>> f = urllib.request.urlopen('http://www.python.org/')
>>> print(f.read(100).decode('utf-8'))

...
Use of Basic HTTP Authentication:

urllib.request.urlopen('http://www.example.com/login.html')

...
Adding HTTP headers:

r = urllib.request.urlopen(req)

This is a follow-up from ruff #13683.

Linked PRs
@opk12 opk12 added the docs Documentation in the Doc dir label Feb 14, 2025
@rruuaanng
Copy link
Contributor

Image

Are you referring to this?

@picnixz
Copy link
Member
picnixz commented Feb 17, 2025 •
edited
Loading

Although technically possible, this is not recommended.

It's not that it's not recommended, it's just that it would only free resources upon object deletion and not upon request termination. But we should mention that the resources are indeed cleaned up but much later.

EDIT: actually, I've checked and it's not even freed upon object deletion! I couldn't find any __del__ implementation which calls .close() so it's indeed better not to recommend it, or add an explicit close() call.

@picnixz picnixz changed the title [documentation] urlopen examples without with docs: add an explicit close() call to urlopen examples without with Feb 17, 2025
Mr-Sunglasses added a commit to Mr-Sunglasses/cpython that referenced this issue Feb 18, 2025
@Mr-Sunglasses
pythongh-130132: add an explicit close() call to urlopen examples wit…
acafffd 
…hout with
@Mr-Sunglasses Mr-Sunglasses mentioned this issue Feb 18, 2025
Merged
@opk12
Copy link
Author
opk12 commented Feb 18, 2025

@Mr-Sunglasses @rruuaanng Proper closing should be added to all 3 quoted examples. As you may know, the audience includes learners and people from other fields than programming.

@opk12
Copy link
Author
opk12 commented Feb 18, 2025

To be honest, I'm not sure that the examples are worth keeping? Using a context manager without with deserves a bit of explanation and IMO the examples make a point that feels out of place in the docs for urllib.

Mr-Sunglasses added a commit to Mr-Sunglasses/cpython that referenced this issue Feb 18, 2025
@Mr-Sunglasses
pythongh-130132: add an explicit close() in Use of Basic HTTP Authent…
3c00f5d 
…ication
Mr-Sunglasses added a commit to Mr-Sunglasses/cpython that referenced this issue Feb 18, 2025
@Mr-Sunglasses
pythongh-130132: add an explicit close() in Adding HTTP headers
379b075
@Mr-Sunglasses
Copy link
Contributor

@Mr-Sunglasses @rruuaanng Proper closing should be added to all 3 quoted examples. As you may know, the audience includes learners and people from other fields than programming.

Thanks a lot @opk12 for suggestion, I've added explicit close() in all the remaining examples.

@Mr-Sunglasses
Copy link
Contributor

To be honest, I'm not sure that the examples are worth keeping? Using a context manager without with deserves a bit of explanation and IMO the examples make a point that feels out of place in the docs for urllib.

I think the examples are pretty outdated, probably from the time when with does not exists ( completely agree on this

Using a context manager without with deserves a bit of explanation

and Yeah I agree as it feel very outdated, we need to work on them, as most of the examples does not work or the information is outdates.

let's wait for the opinion of core dev's, what they think about this issue.

@Mr-Sunglasses Mr-Sunglasses mentioned this issue Feb 18, 2025
Closed
picnixz added a commit that referenced this issue Mar 18, 2025
@Mr-Sunglasses @sobolevn @picnixz
gh-130132: properly free resources in urrlib.urlopen examples (#130280
77d2fd4 
)

Co-authored-by: sobolevn <mail@sobolevn.me>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Mar 18, 2025
@Mr-Sunglasses @sobolevn
@picnixz @miss-islington
pythongh-130132: properly free resources in urrlib.urlopen examples (…
0a50fc6 
…pythonGH-130280)

(cherry picked from commit 77d2fd4)

Co-authored-by: Kanishk Pachauri <itskanishkp.py@gmail.com>
Co-authored-by: sobolevn <mail@sobolevn.me>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Mar 18, 2025
@Mr-Sunglasses @sobolevn
@picnixz @miss-islington
pythongh-130132: properly free resources in urrlib.urlopen examples (…
2eb4bdf 
…pythonGH-130280)

(cherry picked from commit 77d2fd4)

Co-authored-by: Kanishk Pachauri <itskanishkp.py@gmail.com>
Co-authored-by: sobolevn <mail@sobolevn.me>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
This was referenced Mar 18, 2025
Merged
Merged
@picnixz
Copy link
Member
picnixz commented Mar 18, 2025

Thank you for the report and the fix!

picnixz added a commit that referenced this issue Mar 18, 2025
@miss-islington @Mr-Sunglasses
@sobolevn @picnixz
[3.13] gh-130132: properly free resources in urrlib.urlopen examples (
f297dab 
GH-130280) (#131394)

gh-130132: properly free resources in `urrlib.urlopen` examples (GH-130280)
(cherry picked from commit 77d2fd4)

Co-authored-by: Kanishk Pachauri <itskanishkp.py@gmail.com>
Co-authored-by: sobolevn <mail@sobolevn.me>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
picnixz added a commit that referenced this issue Mar 18, 2025
@miss-islington @Mr-Sunglasses
@sobolevn @picnixz
[3.12] gh-130132: properly free resources in urrlib.urlopen examples (
2601dae 
GH-130280) (#131395)

gh-130132: properly free resources in `urrlib.urlopen` examples (GH-130280)
(cherry picked from commit 77d2fd4)

Co-authored-by: Kanishk Pachauri <itskanishkp.py@gmail.com>
Co-authored-by: sobolevn <mail@sobolevn.me>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
colesbury pushed a commit to colesbury/cpython that referenced this issue Mar 20, 2025
@Mr-Sunglasses @sobolevn
@picnixz @colesbury
pythongh-130132: properly free resources in urrlib.urlopen examples (…
985cd4e 
…python#130280)

Co-authored-by: sobolevn <mail@sobolevn.me>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
seehwan pushed a commit to seehwan/cpython that referenced this issue Apr 16, 2025
@Mr-Sunglasses @sobolevn
@picnixz @seehwan80
pythongh-130132: properly free resources in urrlib.urlopen examples (…
f038afd 
…python#130280)

Co-authored-by: sobolevn <mail@sobolevn.me>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
docs Documentation in the Doc dir
Projects
docs issues
Status: Todo
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

gh-130132: add an explicit close() call to urlopen examples without with Mr-Sunglasses/cpython
4 participants
@picnixz @opk12 @Mr-Sunglasses @rruuaanng
0