feat(experimentation): experiment rollout with multivariate segment override#7851
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
api
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7851 +/- ##
========================================
Coverage 98.59% 98.60%
========================================
Files 1472 1473 +1
Lines 57362 57688 +326
========================================
+ Hits 56556 56882 +326
Misses 806 806 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
/gemini review |
gagantrivedi
force-pushed
the
feat/experiment-rollout
branch
from
1b7192a to
b342fae
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for experiment rollouts by adding a rollout_segment to the Experiment model, implementing rollout creation and update services, and extending the versioning service to handle multivariate value updates. The review feedback highlights several critical issues: a truthiness check in the serializer prevents clearing multivariate values with an empty list; the rollout update service lacks a database transaction, risking data inconsistency; and the multivariate update loop is susceptible to duplicate creation or integrity errors if duplicate option IDs are provided.
github-actions
Bot
added
feature
gagantrivedi
force-pushed
the
feat/experiment-rollout
branch
from
461a758 to
f90171e
Compare
github-actions
Bot
added
feature
gagantrivedi
force-pushed
the
feat/experiment-rollout
branch
from
f90171e to
851a93b
Compare
github-actions
Bot
added
feature
gagantrivedi
force-pushed
the
feat/experiment-rollout
branch
from
851a93b to
b72bb67
Compare
github-actions
Bot
removed
feature
Docker builds report
|
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
|
Visual Regression19 screenshots compared. See report for details. |
Zaimwa9
left a comment
Zaimwa9
left a comment
There was a problem hiding this comment.
Couple of comments, most important one to me is the transaction in the update path. And maybe be intentional on how we set the priority for these segments
Address review: the percentage-split condition save and the flag-state update in update_experiment_rollout are now atomic, so a failure in the flag update no longer leaves the split percentage updated on its own.
github-actions
Bot
added
feature
… via one operation Address review: replace the to_service_kwargs/**kwargs seam with a typed RolloutSpec DTO, collapse create/update into a single apply_experiment_rollout (with a private _sync_rollout_segment hiding the create-or-mutate decision), and consolidate rollout validation into one validate_rollout_spec run before any side effect.
github-actions
Bot
added
feature
for more information, see https://pre-commit.ci
4 participants
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Adds an experiment rollout to experiment creation. Posting an
experiment_rolloutblock alongside a new experiment provisions, in one atomic create:PERCENTAGE_SPLITsystem segment (is_system_segment=True, so it's hidden from the segments API/UI and audit log) gating the experiment cohort byrollout_percentage;multivariate_feature_state_valuessplit.Supporting change in the versioning layer:
FlagChangeSet/SegmentOverrideChangeSetandupdate_flag/update_flag_v2now carry optional multivariate allocations (upsert-style), so segment-override writes set the variant split correctly for both v1 and v2 feature-versioning environments. This also benefits the existingupdate-flag-v2endpoint.Not yet included (intentional)
experiment_rolloutis write-only — no read representation for the UI yet.How did you test this code?
Unit tests (all green locally), plus
mypy,ruff, and the test linter:update_flag/update_flag_v2set/upsert/remove multivariate allocations on segment overrides, for v1 and v2 environments.create_experiment_rolloutbuilds the system %-split segment + override;update_experiment_rolloutupdates it and is blocked while running/completed and when no rollout exists.experiment_rollout(incl. invalid MV option → 400), therolloutPATCH action (happy path, running → 400, invalid MV → 400), and rejecting a rollout sent to the experiment update endpoint.