Skip to content

chore: allow using code defaults with feature flags#1991

Open
jakubno wants to merge 3 commits intomainfrom
chore/allow-using-code-defaults-with-ff
Open

chore: allow using code defaults with feature flags#1991
jakubno wants to merge 3 commits intomainfrom
chore/allow-using-code-defaults-with-ff

Conversation

@jakubno
Copy link
Member

@jakubno jakubno commented Feb 25, 2026

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: typedFlag now exposes shouldUseFallback, and getFlag returns the flag’s fallback when LaunchDarkly returns a sentinel value. Introduces JSONFlagSentinel plus newJSONFlagWithFallback to enable this behavior for selected JSON flags (currently FirecrackerVersions), 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.

@jakubno jakubno marked this pull request as ready for review February 25, 2026 12:47
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

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.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 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
Copy link
Member

Choose a reason for hiding this comment

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

Not sure about naming, it not self explaining imho

Copy link
Member

@sitole sitole left a comment

Choose a reason for hiding this comment

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

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?

@djeebus
Copy link
Contributor

djeebus commented Feb 25, 2026

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?

@jakubno
Copy link
Member Author

jakubno commented Feb 25, 2026

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

@jakubno jakubno force-pushed the chore/allow-using-code-defaults-with-ff branch from c7eba08 to bd63cd2 Compare February 28, 2026 07:38
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.

4 participants