Skip to content

feat(__future__): Experimental update-flag - #8102

Draft
emyller wants to merge 4 commits into
mainfrom
feat/update-flag-option-c
Draft

feat(__future__): Experimental update-flag#8102
emyller wants to merge 4 commits into
mainfrom
feat/update-flag-option-c

Conversation

@emyller

@emyller emyller commented Jul 25, 2026

Copy link
Copy Markdown
Contributor
  • I have read the Contributing Guide.
  • I have added information to docs/ if required so people know about the feature.
  • I have filled in the "Changes" section below.
  • I have filled in the "How did you test this code" section below.

Changes

Contributes to #7642

Stabilises experimental update-flag endpoints to one option, still experimental.

  • Avoids touching Options A + B — cleanup + interface migration PR will follow.
  • Foundation of the code is in an experimental namespace.
  • Includes key-based multivariate flag support.
  • Improves API design.

How did you test this code?

Integration tests included.

@emyller emyller self-assigned this Jul 25, 2026
@vercel

vercel Bot commented Jul 25, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview Aug 4, 2026 12:24am
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
flagsmith-frontend-preview Ignored Ignored Preview Aug 4, 2026 12:24am
flagsmith-frontend-staging Ignored Ignored Preview Aug 4, 2026 12:24am

Request Review

@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4a11882d-a2fc-4bdc-adff-0c973e1336d0

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added api Issue related to the REST API docs Documentation updates feature New feature or request and removed docs Documentation updates labels Jul 25, 2026

@khvn26 khvn26 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good overall. My main gripe is combining full-replace and delete: true semantics for lists of different entities in a single API.


### Configure multiple segment overrides

Set different values per segment — for example, pricing tiers.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can I omit priorities in this case?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd say that we should infer priorities from the ordering in the payload if not provided.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Which priority numbers could represent ordering in the payload? Are we thinking of something close to INF - n, or a regular 1-based enumeration?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there any difference?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

My initial instinct was to steer away from user input space, but I guess it mostly doesn't matter since we reorder in the UI using drag and drop.

@emyller emyller Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should be improved in 98593e0.

"segment_overrides": [
{
"segment_id": 456,
"delete": true

@khvn26 khvn26 Jul 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not loving the API here. I'd prefer to have full replace PUT semantics instead — demand the full overrides list, delete omitted elements.

IMO, individual override operations work well with individual APIs, e.g.

  • DELETE /api/experiments/environments/{environment_key}/update-flag/{feature_id}/segment/{segment_id}
  • PUT /api/experiments/environments/{environment_key}/update-flag/{feature_id}/identity/{identifier}

This option especially makes sense in the CLI V2 context.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd prefer to have full replace PUT semantics instead — demand the full overrides list, delete omitted elements.

This sounds like a can of worms that leads back to needing both option_a and _b. I do agree that I don't like the idea of adding the new "delete": true attribute to the payload, however. I prefer a separate call for this. We had (I think) a /remove-segment-override endpoint previously alongside the /update-flag one, right? I tend to think that Kim's suggestion is a good one though of having DELETE /api/experiments/environments/{environment_key}/update-flag/{feature_id}/segment/{segment_id} .

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

IMO, individual override operations work well with individual APIs, e.g.

  • DELETE /api/experiments/environments/{environment_key}/update-flag/{feature_id}/segment/{segment_id}
  • PUT /api/experiments/environments/{environment_key}/update-flag/{feature_id}/identity/{identifier}

Love it. For some reason, I thought we were constrained to one swiss-army-knifey /update-flag/ endpoint only.

@emyller emyller Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

demand the full overrides list, delete omitted elements.

I think this would break the intended ergonomics of this endpoint — change only these things about this feature, leave out all the rest. I think this intent is only inheritance of Option B's nature, which we chose.

On a second thought, the suggested /update-flag/{feature_id}/ path also breaks ergonomics since it relies on the feature ID, while this endpoint assumes the user may choose to identify features given their name.

So, should we:

  1. Support either a feature name or ID in the URL?
  2. Choose to support only feature_name?
  3. Restore Gagan's separate POST .../delete-segment-override/ but with Option B's style?

Set different values per segment — for example, pricing tiers.

```bash
curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environment_key}/update-flag/' \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: Since we're shipping experiments, I think it's best to avoid confusion:

Suggested change
curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environment_key}/update-flag/' \
curl -X POST 'https://api.flagsmith.com/api/experimental/environments/{environment_key}/update-flag/' \

But maybe we should bite the bullet already:

Suggested change
curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environment_key}/update-flag/' \
curl -X POST 'https://api.flagsmith.com/api/v2/environments/{environment_key}/update-flag/' \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We don't need v2 though because we're not changing an endpoint - this is a new endpoint. It's under the experiments namespace because it's subject to change, not because it's any sort of breaking change. If we go with v2 and need to change it, we'll have to go to v3 ...

I do agree with the comment on experiments vs experimental though. It might be a PITA to change that now though.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Arguably, this is the first time this experiments/ namespace is used, and we're updating the URLs anyway, so it should be an easy change.

/api/__future__/environments/{environment_key}/update-flag/{feature_name}

I've borrowed Python's dunder-future name in the pull request title. Not sure if a good disambiguation suggestion for an API path, but it is definitely one.

Comment on lines +137 to +138
The `variants` list in `segment_overrides` can only re-weight existing variants. Variants omitted from it keep their
current weights for that segment.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The variants list in segment_overrides can only re-weight existing variants.

👍

Variants omitted from it keep their current weights for that segment.

I am not exactly sure how that's supposed to work. For example, if we send 1 variant with 100% weight, the backend ought to re-weight all other variants to 0?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we instead enforce that the client has to provide all variants?

@emyller emyller Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Improved in e111394.

Set different values per segment — for example, pricing tiers.

```bash
curl -X POST 'https://api.flagsmith.com/api/experiments/environments/{environment_key}/update-flag/' \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We don't need v2 though because we're not changing an endpoint - this is a new endpoint. It's under the experiments namespace because it's subject to change, not because it's any sort of breaking change. If we go with v2 and need to change it, we'll have to go to v3 ...

I do agree with the comment on experiments vs experimental though. It might be a PITA to change that now though.


### Configure multiple segment overrides

Set different values per segment — for example, pricing tiers.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd say that we should infer priorities from the ordering in the payload if not provided.


Set up features with weighted variants and customise weights per segment.

The `variants` list in `environment_default` defines the available variants for the feature, and their default weights

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure I agree that modifying the variants themselves should be part of this endpoint at all. variants (or MultivariateFeatureOptions) are project level concepts. This endpoint should be concerned only with environment level entities.

IMO we should raise an error if a variant key is provided that doesn't already exist.

@khvn26 khvn26 Jul 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I actually thought this was implied by

The variants list in segment_overrides can only re-weight existing variants.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That's not how I understood it - I understood that there was a difference in behaviour between the environment_default section of the payload, and the segment_overrides on this matter. TBC by @emyller .

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

My honest understanding of adding MVs support to update-flag was that it could be used to configure a multivariate experiment. If it's not, I think I need to learn more about the use case.

@emyller emyller Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Leaning towards knowledge I'm yet to learn: e111394

Comment on lines +137 to +138
The `variants` list in `segment_overrides` can only re-weight existing variants. Variants omitted from it keep their
current weights for that segment.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we instead enforce that the client has to provide all variants?

"segment_overrides": [
{
"segment_id": 456,
"delete": true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd prefer to have full replace PUT semantics instead — demand the full overrides list, delete omitted elements.

This sounds like a can of worms that leads back to needing both option_a and _b. I do agree that I don't like the idea of adding the new "delete": true attribute to the payload, however. I prefer a separate call for this. We had (I think) a /remove-segment-override endpoint previously alongside the /update-flag one, right? I tend to think that Kim's suggestion is a good one though of having DELETE /api/experiments/environments/{environment_key}/update-flag/{feature_id}/segment/{segment_id} .

@emyller
emyller force-pushed the feat/update-flag-option-c branch from 2b9f91b to 4593ce3 Compare August 3, 2026 22:43
@github-actions github-actions Bot added the docs Documentation updates label Aug 3, 2026
@github-actions github-actions Bot added feature New feature or request and removed feature New feature or request docs Documentation updates labels Aug 3, 2026
@emyller
emyller force-pushed the feat/update-flag-option-c branch from 4593ce3 to ff05fcf Compare August 4, 2026 00:17
@github-actions github-actions Bot added the docs Documentation updates label Aug 4, 2026
@github-actions github-actions Bot added feature New feature or request docs Documentation updates and removed feature New feature or request docs Documentation updates labels Aug 4, 2026
@emyller
emyller force-pushed the feat/update-flag-option-c branch from c17e1a8 to 468a299 Compare August 4, 2026 00:22
@github-actions github-actions Bot removed feature New feature or request docs Documentation updates labels Aug 4, 2026
@github-actions github-actions Bot added the feature New feature or request label Aug 4, 2026
@emyller
emyller force-pushed the feat/update-flag-option-c branch from 40914ff to e111394 Compare August 4, 2026 00:23
@github-actions github-actions Bot added docs Documentation updates feature New feature or request and removed feature New feature or request docs Documentation updates labels Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api Issue related to the REST API feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants