Skip to content

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

Merged
vstinner merged 6 commits into
python:mainfrom
vstinner:reload_environ
Nov 5, 2024
Merged

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

Conversation

@vstinner

@vstinner vstinner commented Nov 1, 2024

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/

Replace the os.environ.refresh() method with a new
os.reload_environ() function.
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

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.

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 rruuaanng Nov 1, 2024

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.

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.

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.

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

@vstinner

vstinner commented Nov 1, 2024

Copy link
Copy Markdown
Member Author

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

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

LGTM. Let a native speaker to review the wording.

Comment thread Doc/library/os.rst
@vstinner

vstinner commented Nov 1, 2024

Copy link
Copy Markdown
Member Author

cc @zooba @ncoghlan

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

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
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@vstinner

vstinner commented Nov 1, 2024

Copy link
Copy Markdown
Member Author

@picnixz: I applied your suggestions.

Comment thread Doc/library/os.rst Outdated

@ncoghlan 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.

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

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>

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

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
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

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

Looks great to me!

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

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
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
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants