Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.3k
gh-120057: Add os.reload_environ() function#126268
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Replace the os.environ.refresh() method with a newos.reload_environ() function.
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
os.putenv() andos.unsetenv() don't updateos.environ: seetest_reload_environ().
@serhiy-storchaka: Please review the updated PR. I addressed your review. |
There was a problem hiding this comment.
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.
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
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
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@picnixz: I applied your suggestions. |
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
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>
There was a problem hiding this comment.
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.
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Looks great to me!
4a0d574 intopython:mainUh oh!
There was an error while loading.Please reload this page.
Merged, thanks for reviews! |
Replace the os.environ.refresh() method with a newos.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>
Replace the os.environ.refresh() method with a newos.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>
Uh oh!
There was an error while loading.Please reload this page.
Replace the os.environ.refresh() method with a new os.reload_environ() function.
📚 Documentation preview 📚:https://cpython-previews--126268.org.readthedocs.build/