FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-120057: Add os.reload_environ() function by vstinner · Pull Request #126268 · python/cpython · GitHub

/ cpython Public

gh-120057: Add os.reload_environ() function - #126268

Merged
vstinner merged 6 commits into
python:mainfrom
vstinner:reload_environ
Nov 5, 2024
ConversationCommits (6)ChecksFiles changed
Merged

gh-120057: Add os.reload_environ() function#126268
vstinner merged 6 commits into
python:mainfrom
vstinner:reload_environ

Conversation

vstinner commented Nov 1, 2024 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Member

Replace the os.environ.refresh() method with a new os.reload_environ() function.

📚 Documentation preview 📚: https://cpython-previews--126268.org.readthedocs.build/

pythongh-120057: Add os.reload_environ() function
0a00444 
Replace the os.environ.refresh() method with a new
os.reload_environ() function.
bedevere-app Bot mentioned this pull request Nov 1, 2024
Closed
serhiy-storchaka reviewed Nov 1, 2024
View reviewed changes
Comment thread Doc/library/os.rst Outdated
.. function:: reload_environ()

Update :data:`os.environ` and :data:`os.environb` with changes to the
environment made by :func:`os.putenv`, by :func:`os.unsetenv`, or made

serhiy-storchaka Nov 1, 2024

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think that it is better to say "the process environment".
os.putenv and os.unsetenv update the cache, so there is no need to reload after them. You should refer to the C functions.
Please add a note that this function is not thread safe. Calling it while the environment is modified in other thread has undefined behavior. Reading from os.environ or calling os.getenv during reloading can return empty result.

rruuaanng Nov 1, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

If the process environment is mentioned, perhaps it can be specifically mentioned that it is the current process environment (I think)

Edit
Or, it can be called the current program.

vstinner Nov 1, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

os.putenv() and os.unsetenv() don't update os.environ: see test_reload_environ().

vstinner commented Nov 1, 2024

Copy link
Copy Markdown
Member Author

@serhiy-storchaka: Please review the updated PR. I addressed your review.

serhiy-storchaka approved these changes Nov 1, 2024
View reviewed changes

serhiy-storchaka 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM. Let a native speaker to review the wording.

Comment thread Doc/library/os.rst

vstinner commented Nov 1, 2024

Copy link
Copy Markdown
Member Author

cc @zooba @ncoghlan

picnixz reviewed Nov 1, 2024
View reviewed changes

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Not a native speaker but here are some suggestions. For native speakers: @python/proofreaders

Comment thread Doc/library/os.rst Outdated
Comment thread Doc/library/os.rst Outdated
Comment thread Doc/library/os.rst Outdated
Apply suggestions from code review
fde06e9 
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

vstinner commented Nov 1, 2024

Copy link
Copy Markdown
Member Author

@picnixz: I applied your suggestions.

AA-Turner reviewed Nov 1, 2024
View reviewed changes
Comment thread Doc/library/os.rst Outdated
ncoghlan approved these changes Nov 1, 2024
View reviewed changes

ncoghlan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM! (although see @AA-Turner's suggested docs wording tweaks)

Update Doc/library/os.rst
ca90336 
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
picnixz approved these changes Nov 2, 2024
View reviewed changes

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Pro tips from someone who had lots of linter errors due to suggestions in the past: when submitting a suggestion, I usually Ctrl+A (or "Select all" on mobile) to check whether the text has trailing whitespaces or not. This helps reducing linter errors.

Comment thread Doc/library/os.rst Outdated
Update Doc/library/os.rst
1b07ef6 
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
zooba approved these changes Nov 4, 2024
View reviewed changes

zooba 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks great to me!

vstinner merged commit 4a0d574 into python:main Nov 5, 2024
vstinner deleted the reload_environ branch November 5, 2024 07:43

vstinner commented Nov 5, 2024

Copy link
Copy Markdown
Member Author

Merged, thanks for reviews!

picnixz added a commit to picnixz/cpython that referenced this pull request Dec 8, 2024
pythongh-120057: Add os.reload_environ() function (python#126268)
610da8f 
Replace the os.environ.refresh() method with a new
os.reload_environ() function.

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
pythongh-120057: Add os.reload_environ() function (python#126268)
1960b30 
Replace the os.environ.refresh() method with a new
os.reload_environ() function.

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
AlexWaygood mentioned this pull request Oct 29, 2025
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

ncoghlan ncoghlan approved these changes
cmaloney cmaloney approved these changes
zooba zooba approved these changes
serhiy-storchaka serhiy-storchaka approved these changes
AA-Turner AA-Turner approved these changes
picnixz picnixz approved these changes
+1 more reviewer
rruuaanng rruuaanng left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

Back | FazBrowse Home | New Git URL