chore: allow using code defaults with feature flags#1991
chore: allow using code defaults with feature flags#1991
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7eba0853c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| type typedFlag[T any] interface { | ||
| Key() string | ||
| Fallback() T | ||
| isSentinel(value T) bool |
There was a problem hiding this comment.
Not sure about naming, it not self explaining imho
sitole
left a comment
There was a problem hiding this comment.
Can we put a logic comment directly to the getFlag function and explain the case we are trying to solve there?
Also, for bool, there is no "empty value", so you cannot distinguish between an actually configured value and an off (empty) value. Maybe we should deprecate the bool feature flags then?
|
Similar to what @sitole said, and since our deploys are slower than changing flags in launch darkly, I'm not sure I understand the value here. Can you explain the use case? |
|
E.g. for orchestrator we have feature flag for firecracker versions, this is there so we have fast way how to change firecracker version if anything goes wrong, but by default you want to use what's in code. That also allow us to run older version with old FC version, the new version with the new FC version, current setup doesn't allow that |
c7eba08 to
bd63cd2
Compare
Note
Medium Risk
Medium risk because it changes the shared feature-flag evaluation path and can alter runtime behavior for JSON flags when a sentinel value is configured; mistakes could silently revert to defaults. Limited scope via opt-in sentinel and added tests reduce likelihood of regressions.
Overview
Adds opt-in support for "use code default" sentinels in feature flag evaluation:
typedFlagnow exposesshouldUseFallback, andgetFlagreturns the flag’s fallback when LaunchDarkly returns a sentinel value. IntroducesJSONFlagSentinelplusnewJSONFlagWithFallbackto enable this behavior for selected JSON flags (currentlyFirecrackerVersions), and adds tests to ensure sentinel JSON values trigger fallback while other flag types and non-sentinel JSON values pass through unchanged.Written by Cursor Bugbot for commit 05dfdfc. This will update automatically on new commits. Configure here.