Skip to content

Fix required configuration option cache invalidation - #980

Merged
ZipFile merged 1 commit into
ets-labs:developfrom
Kryptonzy:agent/fix-configuration-option-cache-override
Aug 11, 2026
Merged

Fix required configuration option cache invalidation#980
ZipFile merged 1 commit into
ets-labs:developfrom
Kryptonzy:agent/fix-configuration-option-cache-override

Conversation

@Kryptonzy

@Kryptonzy Kryptonzy commented Aug 9, 2026

Copy link
Copy Markdown

Summary

Make a configuration option reuse and track its derived .required() provider so configuration cache resets also invalidate the required view.

Fixes #954.

Root cause and approach

ConfigurationOption.required() created a detached provider each time. After that provider resolved a value, the original option had no reference to its cache. An override correctly reset the root/original option tree, but could not reach the required provider captured by a Singleton or Factory, so recreating the consumer still returned the stale value.

The original option now lazily creates and retains one required provider. reset_cache() cascades to that provider, deepcopy preserves the relationship, and calling required() on an already-required option is idempotent. This keeps the existing cache for performance, as requested in the issue discussion, instead of recomputing every access.

Verification

  • New regression on unmodified code: failed because the recreated singleton received {"value": "initial"} during the override.
  • .venv/bin/pytest -q tests/unit/providers/configuration/test_config_py2_py3.py::test_required_cache_is_reset_after_option_override — 1 passed.
  • .venv/bin/pytest -q tests/unit/providers/configuration — 204 passed.
  • Related Singleton, Factory, and deepcopy test groups — 328 passed.
  • The Cython extension was rebuilt successfully before the green test runs.
  • The changed Python test file passes Flake8; git diff --check passes. (providers.pyx is Cython and is not directly parseable by Flake8.)
  • The remote Tests and linters workflow is currently action_required and generated no jobs. This is the repository's approval gate for workflows from forks, not a test execution failure; no remote-green result is claimed before a maintainer approves it.

Scope

The change is limited to ConfigurationOption cache linkage/copying and one focused regression. It does not remove caching, change configuration lookup rules, or alter Singleton/Factory behavior.

AI assistance

OpenAI Codex (GPT-5) was used to investigate the cache lifecycle, implement the focused change and regression, rebuild the extension, and run/review the verification above. I reviewed the final diff and am disclosing this assistance explicitly.

@Kryptonzy
Kryptonzy marked this pull request as ready for review August 10, 2026 12:44
@Kryptonzy
Kryptonzy marked this pull request as draft August 10, 2026 12:51
@Kryptonzy
Kryptonzy marked this pull request as ready for review August 10, 2026 14:25
@Kryptonzy
Kryptonzy force-pushed the agent/fix-configuration-option-cache-override branch from 9f1500a to 840ac00 Compare August 10, 2026 15:21
@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 92.402%. remained the same — Kryptonzy:agent/fix-configuration-option-cache-override into ets-labs:develop

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

Not exactly fond of this solution, but given that we have to keep backward compatibility this is probably good enough one. For v5 we have to rethink config management for sure.

@ZipFile
ZipFile merged commit 7ae3e1d into ets-labs:develop Aug 11, 2026
14 checks passed
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.

3 participants