Skip to content

DX-119909: support runtime reload of mutable MCP settings#111

Merged
aniket-s-kulkarni merged 12 commits into
dremio:mainfrom
aniket-s-kulkarni:DX-119909-runtime-reload-mutable-mcp-settings
May 19, 2026
Merged

DX-119909: support runtime reload of mutable MCP settings#111
aniket-s-kulkarni merged 12 commits into
dremio:mainfrom
aniket-s-kulkarni:DX-119909-runtime-reload-mutable-mcp-settings

Conversation

@aniket-s-kulkarni
Copy link
Copy Markdown
Contributor

Summary

  • add runtime-mutable metadata and safe reload logic for approved MCP settings
  • split settings storage into process-wide base settings plus request-scoped overrides
  • preserve LaunchDarkly precedence by switching runtime mutable reads to .get(...)
  • broaden the MCP refresh loop to reload settings before reevaluating log_level
  • add focused regression coverage for reload, override isolation, and mutable subtree materialization

Test Plan

  • uv run pytest tests/config/test_settings.py tests/config/test_launchdarkly_integration.py tests/api/test_transport_retry.py tests/api/dremio/test_sql.py tests/servers/test_mcp.py tests/test_fastmcp_basic.py tests/test_simple_fastmcp_server.py tests/tools/test_tools.py tests/tools/test_output_validation.py -q

Jira

Reviewer Verdict

  • Override accepted for the deployment-specific assumption that runtime config entries are not removed after startup

@aniket-s-kulkarni aniket-s-kulkarni force-pushed the DX-119909-runtime-reload-mutable-mcp-settings branch from afc9cee to 0088c8c Compare May 18, 2026 15:40
@aniket-s-kulkarni aniket-s-kulkarni marked this pull request as ready for review May 18, 2026 16:20
Comment thread tests/config/test_settings.py
Comment thread tests/config/test_settings.py
Comment thread src/dremioai/config/settings.py Outdated
Comment thread tests/config/test_launchdarkly_integration.py
Comment thread src/dremioai/config/settings.py
Copy link
Copy Markdown
Contributor

@ssaumitra ssaumitra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Verdict: Issues found — 2 must fix, 3 should fix

Inspect first:

  • src/dremioai/config/settings.py — central reload semantics and restart-required boundary.
  • src/dremioai/servers/mcp.py — applies hot-reloaded log level and logger scope.
  • tests/config/test_settings.py — coverage for startup-only fields and concurrent overrides.
Change walkthrough (19 files)
File Change
src/dremioai/api/dremio/sql.py Reads polling interval through .get(...).
src/dremioai/api/transport.py Reads retry config through .get(...).
src/dremioai/config/settings.py Adds runtime mutability metadata, process-wide base settings, and reload logic.
src/dremioai/log.py Adds scoped logger level support.
src/dremioai/servers/mcp.py Refresh loop reloads settings and updates log levels.
src/dremioai/tools/tools.py Uses LD-aware flag reads and handles missing Dremio config.
tests/api/dremio/test_sql.py Updates polling interval tests.
tests/api/test_transport_retry.py Updates retry config tests.
tests/config/test_launchdarkly_integration.py Adds LD/reload integration coverage.
tests/config/test_settings.py Adds settings reload and override tests.
tests/conftest.py Resets settings state between tests.
tests/servers/test_jwks_verifier.py Adds explicit settings setup.
tests/servers/test_mcp.py Updates settings test helper.
tests/stremable_http_cli.py Updates local server settings propagation.
tests/test_fastmcp_basic.py Updates settings test helper.
tests/test_log.py Adds scoped logging coverage.
tests/test_simple_fastmcp_server.py Updates settings test helper and formatting.
tests/tools/test_output_validation.py Uses scoped settings override helper.
tests/tools/test_tools.py Updates settings helper and formatting.

🔴 Must fix:

  • [Correctness/Architecture] Runtime reload materializes restart-required nested models (src/dremioai/config/settings.py:645).
  • [Architecture] loggers is hot-reloadable but not in the approved mutable scope (src/dremioai/config/settings.py:435).

🟡 Should fix:

  • [Tests] Validation-failure coverage only tests malformed YAML (tests/config/test_launchdarkly_integration.py:672).
  • [Tests] No regression covers tools.server_mode staying startup-only after reload (tests/config/test_settings.py:201).
  • [Tests] Request override isolation is only tested sequentially (tests/config/test_settings.py:94).

Verification: uv run pytest tests/config/test_settings.py tests/config/test_launchdarkly_integration.py -q passed locally: 124 passed, 5 warnings.


Generated by AI-assisted analysis

@aniket-s-kulkarni
Copy link
Copy Markdown
Contributor Author

🔴 Verdict: Issues found — 2 must fix, 3 should fix

Inspect first:

  • src/dremioai/config/settings.py — central reload semantics and restart-required boundary.
  • src/dremioai/servers/mcp.py — applies hot-reloaded log level and logger scope.
  • tests/config/test_settings.py — coverage for startup-only fields and concurrent overrides.
Change walkthrough (19 files)
File Change
src/dremioai/api/dremio/sql.py Reads polling interval through .get(...).
src/dremioai/api/transport.py Reads retry config through .get(...).
src/dremioai/config/settings.py Adds runtime mutability metadata, process-wide base settings, and reload logic.
src/dremioai/log.py Adds scoped logger level support.
src/dremioai/servers/mcp.py Refresh loop reloads settings and updates log levels.
src/dremioai/tools/tools.py Uses LD-aware flag reads and handles missing Dremio config.
tests/api/dremio/test_sql.py Updates polling interval tests.
tests/api/test_transport_retry.py Updates retry config tests.
tests/config/test_launchdarkly_integration.py Adds LD/reload integration coverage.
tests/config/test_settings.py Adds settings reload and override tests.
tests/conftest.py Resets settings state between tests.
tests/servers/test_jwks_verifier.py Adds explicit settings setup.
tests/servers/test_mcp.py Updates settings test helper.
tests/stremable_http_cli.py Updates local server settings propagation.
tests/test_fastmcp_basic.py Updates settings test helper.
tests/test_log.py Adds scoped logging coverage.
tests/test_simple_fastmcp_server.py Updates settings test helper and formatting.
tests/tools/test_output_validation.py Uses scoped settings override helper.
tests/tools/test_tools.py Updates settings helper and formatting.

🔴 Must fix:

  • [Correctness/Architecture] Runtime reload materializes restart-required nested models (src/dremioai/config/settings.py:645).
  • [Architecture] loggers is hot-reloadable but not in the approved mutable scope (src/dremioai/config/settings.py:435).

🟡 Should fix:

  • [Tests] Validation-failure coverage only tests malformed YAML (tests/config/test_launchdarkly_integration.py:672).
  • [Tests] No regression covers tools.server_mode staying startup-only after reload (tests/config/test_settings.py:201).
  • [Tests] Request override isolation is only tested sequentially (tests/config/test_settings.py:94).

Verification: uv run pytest tests/config/test_settings.py tests/config/test_launchdarkly_integration.py -q passed locally: 124 passed, 5 warnings.


Generated by AI-assisted analysis

📬 Review comments addressed — 2026-05-18

Finding Resolution Notes
[Tests] Request override isolation is only tested sequentially (tests/config/test_settings.py:95) ✅ Implemented Added an overlapping asyncio.gather(...) regression that verifies per-task override isolation.
[Tests] No regression covers tools.server_mode staying startup-only (tests/config/test_settings.py:202) ✅ Implemented Added reload coverage showing tools.server_mode and the derived tool set stay unchanged until restart.
[Correctness/Architecture] Runtime reload materializes restart-required nested models (src/dremioai/config/settings.py:645) ✅ Implemented Reload now only recurses into nested models already present in the active base snapshot.
[Tests] Validation-failure coverage only tests malformed YAML (tests/config/test_launchdarkly_integration.py:672) ✅ Implemented Added a valid-YAML invalid-schema regression and verified snapshot + LD state preservation.
[Architecture] loggers is hot-reloadable but not in the approved mutable scope (src/dremioai/config/settings.py:435) ✅ Implemented loggers is startup-only again; only log_level remains hot-reloadable at the top level.

@aniket-s-kulkarni aniket-s-kulkarni enabled auto-merge (squash) May 19, 2026 15:17
@aniket-s-kulkarni aniket-s-kulkarni merged commit bb6f6d7 into dremio:main May 19, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants