Skip to content

feat!: DISABLED is a successful evaluation (still defaults)#395

Merged
toddbaert merged 7 commits into
mainfrom
feat/disabled-evaluator-tests
Jun 2, 2026
Merged

feat!: DISABLED is a successful evaluation (still defaults)#395
toddbaert merged 7 commits into
mainfrom
feat/disabled-evaluator-tests

Conversation

@jonathannorris
Copy link
Copy Markdown
Member

@jonathannorris jonathannorris commented May 25, 2026

  • Disabled flags now resolve successfully with reason=DISABLED instead of errorCode=FLAG_NOT_FOUND. See the ADR.
  • Breaking, but barely: resolved values are unchanged (the caller-provided default is still surfaced).
  • The break is only observable for consumers that inspect reason / errorCode on disabled-flag evaluations.

Related Issues

Closes #393
Part of open-feature/flagd#1965


Warning

This PR depends on #396 (RPC flagd-selector header wiring) merging first; the cross-flagset disabled scenario in the RPC e2e suite will fail until that lands. All other RPC e2e + in-process scenarios pass.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the test-harness subproject commit in providers/openfeature-provider-flagd/openfeature/test-harness to a newer version. There are no review comments, and I have no additional feedback to provide.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 25, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.62%. Comparing base (e1c3c6a) to head (ae05cac).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #395      +/-   ##
==========================================
- Coverage   96.28%   95.62%   -0.66%     
==========================================
  Files          47       24      -23     
  Lines        1750     1029     -721     
==========================================
- Hits         1685      984     -701     
+ Misses         65       45      -20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jonathannorris jonathannorris force-pushed the feat/disabled-evaluator-tests branch from 72c282e to 2006b8f Compare May 25, 2026 20:56
@toddbaert toddbaert changed the title feat(flagd-core): add disabled flag evaluation e2e scenarios feat!: DISABLED is a successful evaluation (still defaults) Jun 1, 2026
@toddbaert toddbaert force-pushed the feat/disabled-evaluator-tests branch from d270c7b to ba08207 Compare June 1, 2026 19:46
@toddbaert toddbaert marked this pull request as ready for review June 1, 2026 20:06
@toddbaert toddbaert requested review from a team as code owners June 1, 2026 20:06
@toddbaert toddbaert self-requested a review June 2, 2026 11:59
Copy link
Copy Markdown
Member

@toddbaert toddbaert left a comment

Choose a reason for hiding this comment

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

I pushed some changes to finalize this. Note that for the tests to work, we need to merge this.

@JamieSinn
Copy link
Copy Markdown

Looks like a version bump is needed on the package deps.

RuntimeError: The grpc package installed is at version 1.80.0, but the generated code in openfeature/schemas/protobuf/flagd/evaluation/v2/evaluation_pb2_grpc.py depends on grpcio>=1.81.0. Please upgrade your grpc module to grpcio>=1.81.0 or downgrade your generated code using grpcio-tools<=1.80.0.

@toddbaert
Copy link
Copy Markdown
Member

Looks like a version bump is needed on the package deps.

RuntimeError: The grpc package installed is at version 1.80.0, but the generated code in openfeature/schemas/protobuf/flagd/evaluation/v2/evaluation_pb2_grpc.py depends on grpcio>=1.81.0. Please upgrade your grpc module to grpcio>=1.81.0 or downgrade your generated code using grpcio-tools<=1.80.0.

Ya, this is fixed on the other branch I mentioned.

jonathannorris and others added 7 commits June 2, 2026 12:33
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Mirrors the existing reason=DEFAULT substitution: when the server returns an empty variant alongside DISABLED, surface the caller's code default value rather than the zero proto value.

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
MessageToDict drops the value field entirely for DISABLED responses (no value is sent). Fall back to the caller's default_value via .get() instead of raising KeyError; the substitution branch then surfaces the default with reason=DISABLED.

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
v3.8.0 of the test-harness submodule already bundles disabled.feature in evaluator/gherkin/ and the disabled-* flags in evaluator/flags/testkit-flags.json. The hatch build hook auto-copies both into the testkit, so test_evaluator.py picks the disabled scenarios up via get_features_path() without any local files.

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
@toddbaert toddbaert force-pushed the feat/disabled-evaluator-tests branch from ba08207 to ae05cac Compare June 2, 2026 16:33
@toddbaert toddbaert merged commit e7d1d34 into main Jun 2, 2026
13 of 14 checks passed
@toddbaert toddbaert deleted the feat/disabled-evaluator-tests branch June 2, 2026 16:51
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.

feat: flagd-core in-process evaluator returns reason=DISABLED for disabled flags

6 participants