Skip to content

Remove stale values when reloading environment sources - #221

Closed
Imkkey wants to merge 2 commits into
beetbox:mainfrom
Imkkey:fix-env-reload-stale-keys-20261015
Closed

Imkkey wants to merge 2 commits into
beetbox:mainfrom
Imkkey:fix-env-reload-stale-keys-20261015

Conversation

@Imkkey

@Imkkey Imkkey commented Oct 15, 2026 •

Copy link
Copy Markdown

EnvSource.load() updates its existing dictionary without removing keys absent from the new environment. After deleting an environment override and explicitly reloading the source, the deleted value continues to mask lower-priority defaults. Removing all matching variables leaves the entire old source intact.

Clear the source only after the complete replacement environment has been parsed and transformed successfully, then update the same source object. Parsing failures still preserve previous contents. Configuration.reload() remains unchanged and continues to reload YAML sources only.

Regression coverage checks removed overrides and fallback values, missing keys, deletion/recreation, nested values, source identity and settings, ordering with separators disabled, both scalar/full-YAML parsing modes, and preservation on a failed parse.

Validation on Python 3.12.14 against main bf42783:

  • Six regression cases fail before the change; the failed-parse control passes.
  • All 43 environment tests and 306 core tests pass after the change.
  • Full suite: 307 passed, one pre-existing date-dependent release-helper failure. The unchanged base has 300 passes and the same failure (already covered by Make release changelog test independent of the date #211).
  • Ruff lint/format, mypy with the Python 3.10 target, changelog formatting/lint, Sphinx HTML rebuild with warnings as errors, and patch whitespace checks pass.
  • Additional nitpicky Sphinx mode reports the same 144 unresolved-reference warnings on both unchanged base and this branch.

Remote Test and Lint workflows for ef0648a are action_required, awaiting maintainer approval, with zero jobs executed. Remote CI has not passed.

Prepared and checked with OpenAI Codex assistance.

@carlitogwapito

Copy link
Copy Markdown

Test-only commit

I put together a small retained-view regression to go with this fix. It creates a nested view before reloading, removes the environment override and checks that both the retained view and a fresh lookup see the lower-priority fallback. It then recreates the override and checks the retained view again.

The test fails at the fallback assertion on bf427833 and passes on ef0648aa in both parse_yaml_docs modes (Python 3.14.4, PyYAML 6.0.3). I ran just this standalone test, not the full suite. It calls EnvSource.load() directly, so it doesn't extend Configuration.reload() behavior.

If useful, you could add it alongside the source-level reload tests to cover callers that keep a view around. Happy for you to take it or leave it.

Adapt the retained-view regression supplied by @carlitogwapito in commit 11e83d9 into the existing parameterized environment tests. Verify fallback and replacement values through both retained and fresh views, while preserving the source object.

Source: beetbox/confuse#221 (comment)

Imkkey commented Oct 18, 2026

Copy link
Copy Markdown
Author

Thanks @carlitogwapito. Adapted your regression from 11e83d9 into the existing parameterized environment tests in 6f217df, with credit in the commit message. It keeps the same nested view through 4001 → fallback 4000 → 4002 in both parsing modes, and checks fresh lookups and source identity after each reload.

Confirmed that both your original test and the adaptation fail on bf42783 and pass with the fix. On Python 3.12.14 / PyYAML 6.0.3: all 45 environment tests pass; the full suite has 309 passes and the same date-dependent release-helper failure reproduced on the unchanged base. Production code and Configuration.reload() are unchanged.

@Imkkey Imkkey closed this Nov 11, 2026
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.

2 participants