Add unload_dotenv() to reverse load_dotenv for tests - #703
Open
iamsaketh70 wants to merge 1 commit into
Open
iamsaketh70 wants to merge 1 commit into
iamsaketh70 wants to merge 1 commit into
Conversation
Keys are removed from os.environ only when the current value still matches the .env file, so later assignments are left alone. Closes theskumar#594 Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
🟡 Changes recommended
Critical and moderate issues remain in the implementation and formatting.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds public unload_dotenv() functionality to clean environment variables loaded from .env files, with tests and documentation.
Changes:
- Implements conditional environment cleanup.
- Exports and documents the new helper.
- Adds removal, preservation, and missing-file tests.
File summaries
| File | Changes | Final findings |
|---|---|---|
tests/test_main.py |
Adds unload behavior tests. | None |
src/dotenv/main.py |
Implements unload_dotenv(). |
Moderate (3 votes): Interpolation settings do not match default load_dotenv() behavior. Critical (3 votes): Matching values can cause caller-owned variables to be deleted. |
src/dotenv/__init__.py |
Exports the new API. | Critical (3 votes): Import formatting will fail the formatter check. |
README.md |
Documents usage. | None |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| from typing import Any, Optional | ||
|
|
||
| from .main import dotenv_values, find_dotenv, get_key, load_dotenv, set_key, unset_key | ||
| from .main import dotenv_values, find_dotenv, get_key, load_dotenv, set_key, unload_dotenv, unset_key |
Comment on lines
+461
to
+462
| if os.environ.get(key) == value: | ||
| del os.environ[key] |
Comment on lines
+452
to
+456
| values = dotenv_values( | ||
| dotenv_path=dotenv_path, | ||
| stream=stream, | ||
| encoding=encoding, | ||
| ) |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
unload_dotenv()so tests can undoload_dotenv()without leaking env vars into later cases.Fixes #594
Behavior
.envfile asload_dotenv/dotenv_values.os.environonly when the current value still matches the file, so a later explicit assignment is left alone.Trueif at least one variable was removed.Tests
test_unload_dotenv_removes_loaded_values.test_unload_dotenv_keeps_changed_values.test_unload_dotenv_missing_file.pytest tests/test_main.py::test_unload_dotenv_removes_loaded_values tests/test_main.py::test_unload_dotenv_keeps_changed_values tests/test_main.py::test_unload_dotenv_missing_file— 3 passed.