Skip to content

gh-130132: add an explicit close() call to urlopen examples without with - #130280

Merged
picnixz merged 11 commits into
python:mainfrom
Mr-Sunglasses:fix/130132
Mar 18, 2025
Merged

picnixz merged 11 commits into
python:mainfrom
Mr-Sunglasses:fix/130132

Conversation

@Mr-Sunglasses

@Mr-Sunglasses Mr-Sunglasses commented Feb 18, 2025 •

Copy link
Copy Markdown
Contributor

@Mr-Sunglasses

Copy link
Copy Markdown
Contributor Author

@sobolevn @picnixz ptal.

@opk12

opk12 commented Feb 18, 2025

Copy link
Copy Markdown

Thanks for correcting the missing close() bug in one example, there are 2 remaining.

@Mr-Sunglasses

Copy link
Copy Markdown
Contributor Author

Thanks for correcting the missing close() bug in one example, there are 2 remaining.

Thanks @opk12 for suggestions, added it in the remaining examples.

Comment thread Doc/library/urllib.request.rst
Comment thread Doc/library/urllib.request.rst Outdated
@Mr-Sunglasses

Copy link
Copy Markdown
Contributor Author

@sobolevn I have made the requested changes; please review again.

@bedevere-app

bedevere-app Bot commented Feb 20, 2025

Copy link
Copy Markdown

Thanks for making the requested changes!

: please review the changes made to this pull request.

@Mr-Sunglasses

Copy link
Copy Markdown
Contributor Author

@sobolevn I have made the requested changes; please review again.

@bedevere-app

bedevere-app Bot commented Feb 24, 2025

Copy link
Copy Markdown

Thanks for making the requested changes!

: please review the changes made to this pull request.

@sobolevn sobolevn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

let's use 4 spaces

Comment thread Doc/library/urllib.request.rst Outdated
Comment thread Doc/library/urllib.request.rst Outdated
Mr-Sunglasses and others added 2 commits February 26, 2025 01:34

@Mr-Sunglasses Mr-Sunglasses left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@sobolevn I have made the requested changes; please review again.

@Mr-Sunglasses

Copy link
Copy Markdown
Contributor Author

Requesting @sobolevn , @picnixz for review.

@picnixz
picnixz self-requested a review February 28, 2025 21:27

@picnixz picnixz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There are some usages of opener.open (L1319, L1340) that could be in a "with". Please check the entire file with occurrences of open and urlopen. TiA.

@Mr-Sunglasses Mr-Sunglasses left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@picnixz I've make the requested changes. please review again 😄 .

TiA. 🙏🏻

@Mr-Sunglasses
Mr-Sunglasses requested a review from picnixz March 9, 2025 18:20
@picnixz picnixz self-assigned this Mar 12, 2025
Comment thread Doc/library/urllib.request.rst Outdated
Comment thread Doc/library/urllib.request.rst Outdated
Comment thread Doc/library/urllib.request.rst Outdated
Comment thread Doc/library/urllib.request.rst Outdated
Other examples use `f` as the response variable.
@picnixz picnixz added needs backport to 3.12 only security fixes needs backport to 3.13 only security fixes labels Mar 18, 2025
@picnixz
picnixz enabled auto-merge (squash) March 18, 2025 09:23
@picnixz
picnixz merged commit 77d2fd4 into python:main Mar 18, 2025
@github-project-automation github-project-automation Bot moved this from Todo to Done in Docs PRs Mar 18, 2025
@miss-islington-app

Copy link
Copy Markdown

Thanks @Mr-Sunglasses for the PR, and @picnixz for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 18, 2025
…pythonGH-130280)

(cherry picked from commit 77d2fd4)

Co-authored-by: Kanishk Pachauri <[email protected]>
Co-authored-by: sobolevn <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
@bedevere-app

bedevere-app Bot commented Mar 18, 2025

Copy link
Copy Markdown

GH-131394 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.13 only security fixes label Mar 18, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 18, 2025
…pythonGH-130280)

(cherry picked from commit 77d2fd4)

Co-authored-by: Kanishk Pachauri <[email protected]>
Co-authored-by: sobolevn <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
@bedevere-app

bedevere-app Bot commented Mar 18, 2025

Copy link
Copy Markdown

GH-131395 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.12 only security fixes label Mar 18, 2025
picnixz added a commit that referenced this pull request Mar 18, 2025
GH-130280) (#131394)

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

Co-authored-by: Kanishk Pachauri <[email protected]>
Co-authored-by: sobolevn <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
picnixz added a commit that referenced this pull request Mar 18, 2025
GH-130280) (#131395)

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

Co-authored-by: Kanishk Pachauri <[email protected]>
Co-authored-by: sobolevn <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
colesbury pushed a commit to colesbury/cpython that referenced this pull request Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation in the Doc dir skip news

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

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

4 participants