Skip to content

DX-116029: LaunchDarkly feature flag integration#87

Merged
aniket-s-kulkarni merged 24 commits into
dremio:mainfrom
aniket-s-kulkarni:LD-integration
Mar 10, 2026
Merged

DX-116029: LaunchDarkly feature flag integration#87
aniket-s-kulkarni merged 24 commits into
dremio:mainfrom
aniket-s-kulkarni:LD-integration

Conversation

@aniket-s-kulkarni
Copy link
Copy Markdown
Contributor

@aniket-s-kulkarni aniket-s-kulkarni commented Mar 9, 2026 •
edited
Loading

Summary

LaunchDarkly feature flag integration for runtime config overrides.

  • FlagAwareMixin — .get(field) checks LD before returning config value; direct attribute access and model_dump() unaffected
  • NoFlag() annotation — Annotated[type, NoFlag()] excludes fields (credentials, connection URIs) from LD lookups
  • FeatureFlagManager — singleton initialized by Settings.model_post_init, no cyclic imports
  • _propagate_flag_prefixes — auto-sets LD key prefix from model nesting path (e.g. dremio.allow_dml)
  • Golden flag keys — scripts/generate_flag_keys.py generates YAML of all 12 flag keys; test catches accidental renames
  • Periodic log level refresh — asyncio task via FastMCP lifespan syncs log_level from LD every 60s
  • Scoped LD support — only Dremio subtree + log_level are flag-aware; LangChain, BeeAI, Tools, Prometheus use plain BaseModel
  • launchdarkly config — top-level in Settings, env var DREMIOAI_LAUNCHDARKLY__SDK_KEY

Flag keys

dremio.allow_dml, dremio.enable_search, dremio.auth_issuer_uri_override,
dremio.wlm.engine_name, dremio.metrics.enabled, dremio.metrics.port,
dremio.api.polling_interval, dremio.api.http_retry.{max_retries,initial_delay,max_delay,backoff_multiplier},
log_level

Test plan

  • 282 unit/e2e tests passing
  • Golden flag keys test prevents accidental key changes
  • E2e tests: allow_dml and log_level overrides with mocked LD client
  • Log level refresh loop unit tested (updates on change, skips when same)
  • Manual: verify with real LD SDK key in config

🤖 Generated with Claude Code

aniket-s-kulkarni and others added 14 commits March 9, 2026 12:43
@aniket-s-kulkarni
initial changes
c35024e
@aniket-s-kulkarni @claude
DX-116029: Fix LD integration — add enable_search override, fix tests…
bfaea9b 
…, clean up API

- Add enable_search property with LD flag override (matching allow_dml pattern)
- Fix get_bool_flag/get_string_flag signatures (were passing wrong args)
- Fix bare except clause in feature_flags.py
- Rewrite test_launchdarkly_integration.py with proper mocked LD client tests
  (old tests referenced non-existent APIs: _extract_flag_metadata, set_ld_context)
- Remove relay proxy references (not needed yet)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aniket-s-kulkarni @claude
DX-116029: Refactor LD integration — GetterModel/FlagAwareModel with …
29a0380 
…auto prefix propagation

- Introduce GetterModel base with get(field_name) pass-through
- Introduce FlagAwareModel that checks LD singleton before fallback
- Auto-propagate _flag_prefix via Settings.model_post_init tree walk
- Remove _ld_property, raw_allow_dml/raw_enable_search, Dremio.get_flag()
- Remove build_context() from FeatureFlagManager (inlined into get_flag)
- Migrate all sub-models (Wlm, Metrics, HttpRetry, ApiSettings, etc.) to FlagAwareModel
- Update callers (mcp.py, tools.py) to use .get() for LD-aware access
- Add tests for prefix propagation, sub-model overrides, pat/enable_search regression
- Add copyright header to launchdarkly_manual_test.py
- All 270 tests passing

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aniket-s-kulkarni @claude
DX-116029: Use FeatureFlagManager.instance(), top-level import, add d…
33f570a 
…efault param to get()

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aniket-s-kulkarni @claude
Remove _init_flag_manager; FeatureFlagManager.instance() self-initial…
53af9e1 
…izes from settings

FeatureFlagManager.instance() now lazily reads sdk_key from
settings.instance().dremio.launchdarkly instead of requiring it as a parameter.
This removes the eager initialization in Dremio.__init__ and simplifies the
bootstrapping flow.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aniket-s-kulkarni @claude
Remove context builder, trivial docstrings, and try-except in FlagAwa…
ec528cc 
…reModel.get()

- Remove multi-context builder and unused get_bool_flag/get_string_flag from
  FeatureFlagManager (project_id/org_id were never passed by callers)
- FeatureFlagManager.instance() now returns a disabled instance when LD is not
  configured instead of raising ValueError
- Remove try-except ValueError block from FlagAwareModel.get()
- Strip trivial docstrings from settings models and test classes

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aniket-s-kulkarni @claude
Add FlagAwareModel comments, log_level setting, align tests with simp…
7e85c4b 
…lified FeatureFlagManager

- Add block comment on FlagAwareModel explaining how LD flag lookup works
  (prefix propagation, .get() vs direct access, model_dump unaffected)
- Add log_level as top-level Settings field (default "INFO"), LD-overridable
  via flag key "log_level"
- Settings now extends FlagAwareModel so .get() works at the top level
- Update tests to match user's simplified FeatureFlagManager (raises on
  construction failure, no empty-string fallback)
- Add TestLogLevel covering default, config, env, and LD override

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aniket-s-kulkarni @claude
Refactor GetterModel/FlagAwareModel into mixins; .get() raises Attrib…
a889827 
…uteError for invalid fields

Extract GetterMixin and FlagAwareMixin as pure mixins (no BaseModel
inheritance) so they compose cleanly with both BaseModel and BaseSettings.
FlagAwareModel is now just FlagAwareMixin + BaseModel for convenience.
Settings uses FlagAwareMixin + BaseSettings directly, avoiding diamond
inheritance.

.get() now raises AttributeError if the field_name is not a valid attribute
of the model, instead of silently returning None.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aniket-s-kulkarni @claude
Align tests with simplified FlagAwareMixin and safe FeatureFlagManage…
e6f983d 
…r.instance()

- FlagAwareMixin.get() now uses _flag_prefix when calling get_flag
- FeatureFlagManager.instance() gracefully handles missing LD config
  (catches AttributeError/TypeError, passes sdk_key=None)
- Tests updated: singleton test expects disabled manager instead of
  exception, get_flag calls use positional args

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aniket-s-kulkarni @claude
Flatten test classes into plain functions and use parametrize for rep…
f4505a3 
…etitive patterns

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aniket-s-kulkarni @claude
Move launchdarkly config to top-level Settings with default instance
0202901 
launchdarkly is always present (defaulting to LaunchDarkly()), so only
sdk_key can be None. This simplifies FeatureFlagManager.instance() to a
single-line read without try/except.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aniket-s-kulkarni @claude
Add e2e tests for LD feature flags; merge FFM get_flag tests
5c6454c 
- e2e: test allow_dml blocked without LD, allowed with LD override
- e2e: test log_level override via .get() vs direct access
- e2e: test LD flags don't affect config values or model_dump()
- Merge test_ffm_get_flag_returns_ld_value and _not_found into parametrized test

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aniket-s-kulkarni @claude
Simplify _propagate_flag_prefixes: add type hint, use direct assignment
5a408f7 
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aniket-s-kulkarni @claude
Collapse duplicate LD env var tests into parametrized test
4d9da61 
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
maxlepikhin
maxlepikhin reviewed Mar 10, 2026
View reviewed changes
Comment thread src/dremioai/config/feature_flags.py Outdated
Comment thread src/dremioai/config/settings.py
aniket-s-kulkarni and others added 9 commits March 10, 2026 10:34
@aniket-s-kulkarni @claude
Fix cyclic import in feature_flags and add golden flag keys test
a975823 
- Add FeatureFlagManager.initialize() classmethod called by Settings.model_post_init,
  eliminating the need for feature_flags to import settings
- Add scripts/generate_flag_keys.py to generate golden YAML of all flag keys
- Add golden_flag_keys.yaml and test_flag_keys_match_golden to catch accidental
  flag key changes from field renames

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aniket-s-kulkarni @claude
Move collect_flag_keys to settings.py using typing.get_args/get_type_…
6713ff2 
…hints

Consolidate the flag key collection logic as a utility in settings.py,
replacing manual __args__/__origin__ with typing.get_args and get_type_hints.
Script and test now import from settings instead of duplicating the logic.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aniket-s-kulkarni @claude
Add comment explaining Optional unwrap in collect_flag_keys
dfe23f5 
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aniket-s-kulkarni @claude
Remove LD support from deprecated LangChain, BeeAI, and Prometheus mo…
faa1eea 
…dels

Change OpenAi, Ollama, Anthropic, LangChain, and BeeAI from FlagAwareModel
to plain BaseModel since they don't need LaunchDarkly overrides and
LangChain/BeeAI are being deprecated. Prometheus already used BaseModel.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aniket-s-kulkarni @claude
Exclude non-flag-aware BaseModel sub-models from golden flag keys
0029fee 
Skip fields whose type is a BaseModel subclass but not FlagAwareMixin
(e.g. LangChain, BeeAI, LaunchDarkly, Prometheus, OAuth2) since they
are opaque objects, not individual LD-toggleable values.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aniket-s-kulkarni @claude
Exclude credentials and non-flag-aware models from LD flag keys
a80fef6 
- Add _flag_exclude ClassVar to FlagAwareMixin; Dremio excludes uri,
  raw_pat, raw_project_id (connection credentials, not feature flags)
- Change Tools from FlagAwareModel to plain BaseModel
- get() skips LD lookup for excluded fields
- collect_flag_keys() skips excluded fields
- Golden keys reduced to 12 meaningful feature flags

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aniket-s-kulkarni @claude
Use Annotated[..., NoFlag()] for flag exclusion; add periodic log lev…
4e51e2a 
…el refresh

- Replace _flag_exclude ClassVar with NoFlag() annotation marker on fields.
  Pydantic exposes Annotated metadata in field_info.metadata, so both
  get() and collect_flag_keys() check for NoFlag without extra introspection.
- Add _start_log_level_refresh() daemon thread that syncs log level from
  LD flags every 60 seconds while the MCP server is running.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aniket-s-kulkarni @claude
Replace log level refresh thread with asyncio task via FastMCP lifespan
7ba5c25 
Use FastMCP's lifespan context manager to run the periodic log level
refresh as an asyncio task instead of a daemon thread, avoiding thread
overhead and keeping everything in the same event loop.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aniket-s-kulkarni @claude
Add unit tests for periodic log level refresh loop
b386ec7 
Test that the async refresh loop updates logging level when LD returns
a different value, and skips set_level when the level hasn't changed.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aniket-s-kulkarni @claude
Switch generate_flag_keys.py to typer; update help to use uv run
1365fee 
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aniket-s-kulkarni aniket-s-kulkarni enabled auto-merge (squash) March 10, 2026 16:16
@aniket-s-kulkarni aniket-s-kulkarni merged commit 59b7d8c into dremio:main Mar 10, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@maxlepikhin maxlepikhin maxlepikhin approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@aniket-s-kulkarni @maxlepikhin