From 7c3d4fa47de08b8ade88ab060069ae40848a5f67 Mon Sep 17 00:00:00 2001 From: Gagan Trivedi Date: Tue, 23 Jun 2026 12:25:33 +0530 Subject: [PATCH 1/5] feat(experimentation): add experiment rollout with multivariate segment override Add an experiment_rollout block to experiment creation that provisions a percentage-split system segment plus a segment override (with multivariate allocations) on the experiment's feature, and a PATCH rollout action to update it. Extends the versioning service so single and bulk flag updates can carry multivariate values for v1 and v2 environments. --- .../migrations/0009_add_rollout_segment.py | 26 ++ api/experimentation/models.py | 7 + api/experimentation/serializers.py | 66 ++++ api/experimentation/services.py | 87 +++++ api/experimentation/views.py | 22 +- api/features/feature_states/serializers.py | 54 ++++ api/features/versioning/dataclasses.py | 8 + api/features/versioning/versioning_service.py | 47 ++- api/tests/unit/experimentation/conftest.py | 27 ++ .../experimentation/test_experiment_views.py | 264 ++++++++++++++- .../unit/experimentation/test_services.py | 151 +++++++++ .../feature_states/test_serializers.py | 141 ++++++++ ...test_unit_versioning_versioning_service.py | 306 ++++++++++++++++++ .../observability/_events-catalogue.md | 8 +- 14 files changed, 1207 insertions(+), 7 deletions(-) create mode 100644 api/experimentation/migrations/0009_add_rollout_segment.py diff --git a/api/experimentation/migrations/0009_add_rollout_segment.py b/api/experimentation/migrations/0009_add_rollout_segment.py new file mode 100644 index 000000000000..f1a3427ea6c1 --- /dev/null +++ b/api/experimentation/migrations/0009_add_rollout_segment.py @@ -0,0 +1,26 @@ +# Generated by Django 5.2.14 on 2026-06-19 09:59 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("experimentation", "0008_experiment_results"), + ("segments", "0030_add_default_to_segment_version"), + ] + + operations = [ + migrations.AddField( + model_name="experiment", + name="rollout_segment", + field=models.OneToOneField( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="experiment_rollout", + to="segments.segment", + ), + ), + ] diff --git a/api/experimentation/models.py b/api/experimentation/models.py index fbc73cfbd562..0ca9d97475d2 100644 --- a/api/experimentation/models.py +++ b/api/experimentation/models.py @@ -127,6 +127,13 @@ class Experiment(LifecycleModelMixin, SoftDeleteExportableModel): # type: ignor updated_at = models.DateTimeField(auto_now=True) started_at = models.DateTimeField(null=True, blank=True) ended_at = models.DateTimeField(null=True, blank=True) + rollout_segment = models.OneToOneField( + "segments.Segment", + on_delete=models.SET_NULL, + related_name="experiment_rollout", + null=True, + blank=True, + ) class Meta: constraints = [ diff --git a/api/experimentation/serializers.py b/api/experimentation/serializers.py index 530d2970c248..89642c9fb1b9 100644 --- a/api/experimentation/serializers.py +++ b/api/experimentation/serializers.py @@ -4,6 +4,7 @@ from django.db.models import QuerySet from rest_framework import serializers +from core.dataclasses import AuthorData from environments.models import Environment from experimentation.dataclasses import WarehouseEventStats from experimentation.metric_definitions import validate_metric_definition @@ -18,14 +19,21 @@ WarehouseConnection, WarehouseType, ) +from experimentation.services import create_experiment_rollout from experimentation.types import ( SNOWFLAKE_DEFAULTS, MetricExperimentResult, SnowflakeConfig, ) +from features.feature_states.serializers import ( + FeatureValueSerializer, + MultivariateValueSerializer, + validate_multivariate_state_values, +) from features.feature_types import MULTIVARIATE from features.models import Feature from features.multivariate.serializers import NestedMultivariateFeatureOptionSerializer +from features.versioning.dataclasses import MultivariateValueChangeSet class WarehouseConnectionSerializer(serializers.ModelSerializer): # type: ignore[type-arg] @@ -207,6 +215,35 @@ class ExperimentMetricInlineSerializer(serializers.Serializer): # type: ignore[ expected_direction = serializers.ChoiceField(choices=ExpectedDirection.choices) +class ExperimentRolloutSerializer(serializers.Serializer): # type: ignore[type-arg] + enabled = serializers.BooleanField(required=True) + rollout_percentage = serializers.FloatField( + required=True, min_value=0, max_value=100 + ) + feature_state_value = FeatureValueSerializer(required=True) + multivariate_feature_state_values = MultivariateValueSerializer( + many=True, required=False + ) + + @staticmethod + def to_service_kwargs(data: dict[str, Any], request: Any) -> dict[str, Any]: + value = data["feature_state_value"] + return { + "enabled": data["enabled"], + "rollout_percentage": data["rollout_percentage"], + "feature_state_value": value["value"], + "value_type": value["type"], + "multivariate_values": [ + MultivariateValueChangeSet( + multivariate_feature_option_id=mv["multivariate_feature_option"], + percentage_allocation=mv["percentage_allocation"], + ) + for mv in data.get("multivariate_feature_state_values", []) + ], + "author": AuthorData.from_request(request), + } + + class ExperimentSerializer(serializers.ModelSerializer): # type: ignore[type-arg] # Annotated with the common base type so ExperimentListSerializer can # override the field with a read-only representation. @@ -215,6 +252,7 @@ class ExperimentSerializer(serializers.ModelSerializer): # type: ignore[type-ar required=False, write_only=True, ) + experiment_rollout = ExperimentRolloutSerializer(required=False, write_only=True) class Meta: model = Experiment @@ -225,6 +263,7 @@ class Meta: "hypothesis", "status", "metrics", + "experiment_rollout", "created_at", "updated_at", "started_at", @@ -260,9 +299,28 @@ def validate(self, attrs: dict[str, Any]) -> dict[str, Any]: raise serializers.ValidationError( {"metrics": "Cannot change the metrics of an existing experiment."} ) + if self.instance is not None and "experiment_rollout" in attrs: + raise serializers.ValidationError( + { + "experiment_rollout": ( + "Cannot change the rollout via this endpoint; " + "use the rollout endpoint instead." + ) + } + ) self._validate_metrics(attrs.get("metrics") or []) + self._validate_rollout(attrs) return attrs + def _validate_rollout(self, attrs: dict[str, Any]) -> None: + rollout = attrs.get("experiment_rollout") + feature = attrs.get("feature") + if not rollout or feature is None: + return + validate_multivariate_state_values( + feature, rollout.get("multivariate_feature_state_values", []) + ) + def _validate_metrics(self, metrics: list[dict[str, Any]]) -> None: metric_ids = [entry["metric"].id for entry in metrics] if len(metric_ids) != len(set(metric_ids)): @@ -272,6 +330,7 @@ def _validate_metrics(self, metrics: list[dict[str, Any]]) -> None: def create(self, validated_data: dict[str, Any]) -> Experiment: metrics: list[dict[str, Any]] = validated_data.pop("metrics", []) + rollout: dict[str, Any] | None = validated_data.pop("experiment_rollout", None) with transaction.atomic(): experiment: Experiment = super().create(validated_data) ExperimentMetric.objects.bulk_create( @@ -282,6 +341,13 @@ def create(self, validated_data: dict[str, Any]) -> Experiment: ) for entry in metrics ) + if rollout is not None: + create_experiment_rollout( + experiment, + **ExperimentRolloutSerializer.to_service_kwargs( + rollout, self.context["request"] + ), + ) return experiment diff --git a/api/experimentation/services.py b/api/experimentation/services.py index 0a56fa27bc18..fb23b88b54b0 100644 --- a/api/experimentation/services.py +++ b/api/experimentation/services.py @@ -10,9 +10,12 @@ from django.conf import settings from django.db.models import Q from django.utils import timezone +from flag_engine.segments.constants import PERCENTAGE_SPLIT +from rest_framework.exceptions import ValidationError from audit.models import AuditLog from audit.related_object_type import RelatedObjectType +from core.dataclasses import AuthorData from experimentation.constants import ( CONTROL_VARIANT_KEY, EXPERIMENT_FLAG, @@ -50,7 +53,10 @@ srm_p_value, ) from features.models import FeatureState +from features.versioning.dataclasses import FlagChangeSet, MultivariateValueChangeSet +from features.versioning.versioning_service import update_flag from integrations.flagsmith.client import get_openfeature_client +from segments.models import Condition, Segment, SegmentRule if typing.TYPE_CHECKING: from collections.abc import Sequence @@ -58,6 +64,7 @@ from experimentation.models import Experiment, Metric, WarehouseConnection from experimentation.types import ExposureGranularity + from features.feature_states.models import FeatureValueType from organisations.models import Organisation from users.models import FFAdminUser @@ -512,6 +519,86 @@ def transition_experiment_status( return experiment +def _create_rollout_segment( + experiment: Experiment, rollout_percentage: float +) -> Segment: + segment: Segment = Segment.objects.create( + name=f"experiment-{experiment.id}-rollout", + project=experiment.feature.project, + is_system_segment=True, + ) + rule = SegmentRule.objects.create(segment=segment, type=SegmentRule.ALL_RULE) + Condition.objects.create( + rule=rule, + operator=PERCENTAGE_SPLIT, + property="$.identity.key", + value=str(rollout_percentage), + ) + return segment + + +def create_experiment_rollout( + experiment: Experiment, + *, + enabled: bool, + rollout_percentage: float, + feature_state_value: str, + value_type: FeatureValueType, + multivariate_values: list[MultivariateValueChangeSet], + author: AuthorData, +) -> None: + segment = _create_rollout_segment(experiment, rollout_percentage) + experiment.rollout_segment = segment + experiment.save() + update_flag( + experiment.environment, + experiment.feature, + FlagChangeSet( + author=author, + enabled=enabled, + feature_state_value=feature_state_value, + type_=value_type, + segment_id=segment.id, + multivariate_values=multivariate_values, + ), + ) + + +def update_experiment_rollout( + experiment: Experiment, + *, + enabled: bool, + rollout_percentage: float, + feature_state_value: str, + value_type: FeatureValueType, + multivariate_values: list[MultivariateValueChangeSet], + author: AuthorData, +) -> None: + if experiment.status in (ExperimentStatus.RUNNING, ExperimentStatus.COMPLETED): + raise ValidationError( + f"Cannot update the rollout of a {experiment.status} experiment." + ) + segment = experiment.rollout_segment + if segment is None: + raise ValidationError("Experiment has no rollout to update.") + + condition = Condition.objects.get(rule__segment=segment, operator=PERCENTAGE_SPLIT) + condition.value = str(rollout_percentage) + condition.save() + update_flag( + experiment.environment, + experiment.feature, + FlagChangeSet( + author=author, + enabled=enabled, + feature_state_value=feature_state_value, + type_=value_type, + segment_id=segment.id, + multivariate_values=multivariate_values, + ), + ) + + def mark_warehouse_pending_connection( connection: WarehouseConnection, ) -> WarehouseConnection: diff --git a/api/experimentation/views.py b/api/experimentation/views.py index 8310289f4c75..d84192eec844 100644 --- a/api/experimentation/views.py +++ b/api/experimentation/views.py @@ -42,6 +42,7 @@ ExperimentListSerializer, ExperimentMetricSerializer, ExperimentResultsSerializer, + ExperimentRolloutSerializer, ExperimentSerializer, MetricSerializer, WarehouseConnectionSerializer, @@ -54,11 +55,15 @@ mark_warehouse_pending_connection, refresh_warehouse_connection_status, transition_experiment_status, + update_experiment_rollout, ) from experimentation.tasks import ( compute_experiment_exposures, compute_experiment_results, ) +from features.feature_states.serializers import ( + validate_multivariate_state_values, +) from users.models import FFAdminUser logger = logging.getLogger(__name__) @@ -176,7 +181,7 @@ def get_serializer_context(self) -> dict[str, Any]: return context def get_serializer_class(self) -> type[BaseSerializer[Experiment]]: - if self.action in ("list", "retrieve", "start", "pause", "complete"): + if self.action in ("list", "retrieve", "start", "pause", "complete", "rollout"): return ExperimentListSerializer return ExperimentSerializer @@ -290,6 +295,21 @@ def pause(self, request: Request, **kwargs: object) -> Response: def complete(self, request: Request, **kwargs: object) -> Response: return self._transition_status(ExperimentStatus.COMPLETED) + @action(detail=True, methods=["patch"]) + def rollout(self, request: Request, **kwargs: object) -> Response: + experiment: Experiment = self.get_object() + serializer = ExperimentRolloutSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + data = serializer.validated_data + validate_multivariate_state_values( + experiment.feature, data.get("multivariate_feature_state_values", []) + ) + update_experiment_rollout( + experiment, + **ExperimentRolloutSerializer.to_service_kwargs(data, request), + ) + return Response(self.get_serializer(experiment).data) + @action(detail=True, methods=["get"]) def exposures(self, request: Request, **kwargs: object) -> Response: experiment: Experiment = self.get_object() diff --git a/api/features/feature_states/serializers.py b/api/features/feature_states/serializers.py index 19c9d0ab1378..df20853d176a 100644 --- a/api/features/feature_states/serializers.py +++ b/api/features/feature_states/serializers.py @@ -1,3 +1,5 @@ +from typing import Any + from rest_framework import serializers from core.dataclasses import AuthorData @@ -6,6 +8,7 @@ from features.versioning.dataclasses import ( FlagChangeSet, FlagChangeSetV2, + MultivariateValueChangeSet, SegmentOverrideChangeSet, ) from features.versioning.versioning_service import ( @@ -128,11 +131,36 @@ class EnvironmentDefaultSerializer(serializers.Serializer): # type: ignore[type value = FeatureValueSerializer(required=True) +class MultivariateValueSerializer(serializers.Serializer): # type: ignore[type-arg] + multivariate_feature_option = serializers.IntegerField(required=True) + percentage_allocation = serializers.FloatField( + required=True, min_value=0, max_value=100 + ) + + +def validate_multivariate_state_values( + feature: Feature, multivariate_values: list[dict[str, Any]] +) -> None: + if not multivariate_values: + return + option_ids = [mv["multivariate_feature_option"] for mv in multivariate_values] + if len(option_ids) != len(set(option_ids)): + raise serializers.ValidationError("Multivariate options must be unique") + valid = set(feature.multivariate_options.values_list("id", flat=True)) + if invalid := set(option_ids) - valid: + raise serializers.ValidationError( + f"Multivariate options {sorted(invalid)} do not belong to the feature" + ) + + class SegmentOverrideSerializer(serializers.Serializer): # type: ignore[type-arg] segment_id = serializers.IntegerField(required=True) priority = serializers.IntegerField(required=False, allow_null=True) enabled = serializers.BooleanField(required=True) value = FeatureValueSerializer(required=True) + multivariate_feature_state_values = MultivariateValueSerializer( + many=True, required=False + ) class UpdateFlagV2Serializer(BaseFeatureUpdateSerializer): @@ -159,6 +187,20 @@ def validate_segment_overrides( return value + def validate(self, data: dict) -> dict: # type: ignore[type-arg] + overrides = data.get("segment_overrides", []) + if any(o.get("multivariate_feature_state_values") for o in overrides): + feature = Feature.objects.filter( + project_id=self.environment.project_id, **data["feature"] + ).first() + if feature is not None: + for override in overrides: + validate_multivariate_state_values( + feature, + override.get("multivariate_feature_state_values", []), + ) + return data + @property def change_set_v2(self) -> FlagChangeSetV2: validated_data = self.validated_data @@ -172,12 +214,24 @@ def change_set_v2(self) -> FlagChangeSetV2: for override_data in segment_overrides_data: value_data = override_data["value"] + multivariate_data = override_data.get("multivariate_feature_state_values") segment_override = SegmentOverrideChangeSet( segment_id=override_data["segment_id"], enabled=override_data["enabled"], feature_state_value=value_data["value"], type_=value_data["type"], priority=override_data.get("priority"), + multivariate_values=[ + MultivariateValueChangeSet( + multivariate_feature_option_id=mv[ + "multivariate_feature_option" + ], + percentage_allocation=mv["percentage_allocation"], + ) + for mv in multivariate_data + ] + if multivariate_data + else None, ) segment_overrides.append(segment_override) diff --git a/api/features/versioning/dataclasses.py b/api/features/versioning/dataclasses.py index dd513f7e091e..65fd78dacf70 100644 --- a/api/features/versioning/dataclasses.py +++ b/api/features/versioning/dataclasses.py @@ -29,6 +29,13 @@ class FlagChangeSet: segment_id: int | None = None segment_priority: int | None = None + multivariate_values: list[MultivariateValueChangeSet] | None = None + + +@dataclass +class MultivariateValueChangeSet: + multivariate_feature_option_id: int + percentage_allocation: float @dataclass @@ -38,6 +45,7 @@ class SegmentOverrideChangeSet: feature_state_value: str type_: FeatureValueType priority: int | None = None + multivariate_values: list[MultivariateValueChangeSet] | None = None @dataclass diff --git a/api/features/versioning/versioning_service.py b/api/features/versioning/versioning_service.py index 531cc9eba6d9..97b29aae2b0e 100644 --- a/api/features/versioning/versioning_service.py +++ b/api/features/versioning/versioning_service.py @@ -3,15 +3,17 @@ from common.core.utils import using_database_replica from django.db.models import Prefetch, Q, QuerySet from django.utils import timezone -from rest_framework.exceptions import NotFound +from rest_framework.exceptions import NotFound, ValidationError from core.dataclasses import AuthorData from environments.models import Environment from features.feature_states.models import FeatureValueType from features.models import Feature, FeatureSegment, FeatureState, FeatureStateValue +from features.multivariate.models import MultivariateFeatureStateValue from features.versioning.dataclasses import ( FlagChangeSet, FlagChangeSetV2, + MultivariateValueChangeSet, ) from features.versioning.exceptions import DirectFeatureStateWriteNotAllowedError from features.versioning.models import EnvironmentFeatureVersion @@ -186,6 +188,7 @@ def _update_flag_for_versioning_v2( change_set.feature_state_value, change_set.type_, ) + _update_multivariate_values(target_feature_state, change_set.multivariate_values) if change_set.segment_id is not None and change_set.segment_priority is not None: _update_segment_priority(target_feature_state, change_set.segment_priority) @@ -238,6 +241,7 @@ def _update_flag_for_versioning_v1( change_set.feature_state_value, change_set.type_, ) + _update_multivariate_values(target_feature_state, change_set.multivariate_values) if change_set.segment_id is not None and change_set.segment_priority is not None: _update_segment_priority(target_feature_state, change_set.segment_priority) @@ -252,6 +256,43 @@ def _update_feature_state_value( fsv.save() +def _update_multivariate_values( + feature_state: FeatureState, + values: list[MultivariateValueChangeSet] | None, +) -> None: + if values is None: + return + + existing = { + mv.multivariate_feature_option_id: mv + for mv in feature_state.multivariate_feature_state_values.all() + } + + passed_option_ids = {value.multivariate_feature_option_id for value in values} + effective_total = sum(value.percentage_allocation for value in values) + sum( + mv.percentage_allocation + for option_id, mv in existing.items() + if option_id not in passed_option_ids + ) + if effective_total > 100: + raise ValidationError( + "Multivariate allocations for the feature state must not exceed " + f"100%, got {effective_total}%." + ) + + for value in values: + mv = existing.get(value.multivariate_feature_option_id) + if mv is None: + MultivariateFeatureStateValue.objects.create( + feature_state=feature_state, + multivariate_feature_option_id=value.multivariate_feature_option_id, + percentage_allocation=value.percentage_allocation, + ) + elif mv.percentage_allocation != value.percentage_allocation: + mv.percentage_allocation = value.percentage_allocation + mv.save() + + def _create_segment_override( feature: Feature, environment: Environment, @@ -333,6 +374,7 @@ def _update_flag_v2_for_versioning_v2( override.feature_state_value, override.type_, ) + _update_multivariate_values(segment_state, override.multivariate_values) if override.priority is not None: _update_segment_priority(segment_state, override.priority) @@ -351,6 +393,7 @@ def _update_flag_v2_for_versioning_v2( override.feature_state_value, override.type_, ) + _update_multivariate_values(segment_state, override.multivariate_values) new_version.publish( published_by=change_set.author.user, @@ -402,6 +445,7 @@ def _update_flag_v2_for_versioning_v1( override.feature_state_value, override.type_, ) + _update_multivariate_values(segment_state, override.multivariate_values) else: assert len(segment_states) == 1 segment_state = list(segment_states.values())[0] @@ -413,6 +457,7 @@ def _update_flag_v2_for_versioning_v1( override.feature_state_value, override.type_, ) + _update_multivariate_values(segment_state, override.multivariate_values) if override.priority is not None: _update_segment_priority(segment_state, override.priority) diff --git a/api/tests/unit/experimentation/conftest.py b/api/tests/unit/experimentation/conftest.py index 16cb83242203..5680ebc17437 100644 --- a/api/tests/unit/experimentation/conftest.py +++ b/api/tests/unit/experimentation/conftest.py @@ -2,6 +2,7 @@ from django.urls import reverse from pytest_mock import MockerFixture +from core.dataclasses import AuthorData from environments.models import Environment from experimentation import ingestion_sync_service from experimentation.models import ( @@ -11,7 +12,11 @@ WarehouseConnection, WarehouseType, ) +from experimentation.services import create_experiment_rollout from features.models import Feature +from features.multivariate.models import MultivariateFeatureOption +from features.versioning.dataclasses import MultivariateValueChangeSet +from users.models import FFAdminUser @pytest.fixture(autouse=True) @@ -61,3 +66,25 @@ def experiment( status=ExperimentStatus.CREATED, ) return experiment + + +@pytest.fixture() +def experiment_with_rollout( + experiment: Experiment, + multivariate_options: list[MultivariateFeatureOption], + admin_user: FFAdminUser, +) -> Experiment: + option_a, option_b, _ = multivariate_options + create_experiment_rollout( + experiment, + enabled=True, + rollout_percentage=20.0, + feature_state_value="control", + value_type="string", + multivariate_values=[ + MultivariateValueChangeSet(option_a.id, 50.0), + MultivariateValueChangeSet(option_b.id, 50.0), + ], + author=AuthorData(user=admin_user), + ) + return experiment diff --git a/api/tests/unit/experimentation/test_experiment_views.py b/api/tests/unit/experimentation/test_experiment_views.py index 5015500b0d1a..293f9e0b282e 100644 --- a/api/tests/unit/experimentation/test_experiment_views.py +++ b/api/tests/unit/experimentation/test_experiment_views.py @@ -33,7 +33,11 @@ from experimentation.serializers import ExperimentFeatureSerializer from features.feature_types import MULTIVARIATE from features.models import Feature, FeatureState -from features.multivariate.models import MultivariateFeatureStateValue +from features.multivariate.models import ( + MultivariateFeatureOption, + MultivariateFeatureStateValue, +) +from segments.models import Condition from tests.types import EnableFeaturesFixture if TYPE_CHECKING: @@ -1751,3 +1755,261 @@ def test_experiment_feature_serializer__no_env_feature_state__raises( # When / Then with pytest.raises(ValueError, match="No environment feature state found"): serializer.data + + +def test_post__with_experiment_rollout__creates_rollout( + admin_client_new: APIClient, + environment: Environment, + multivariate_feature: Feature, + multivariate_options: list[MultivariateFeatureOption], + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + option_a, option_b, _ = multivariate_options + + # When + response = admin_client_new.post( + _list_url(environment), + data={ + "feature": multivariate_feature.id, + "name": "Rollout experiment", + "hypothesis": "It will work", + "experiment_rollout": { + "enabled": True, + "rollout_percentage": 30, + "feature_state_value": {"type": "string", "value": "control"}, + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": option_a.id, + "percentage_allocation": 60, + }, + { + "multivariate_feature_option": option_b.id, + "percentage_allocation": 40, + }, + ], + }, + }, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_201_CREATED + experiment = Experiment.objects.get(id=response.json()["id"]) + assert experiment.rollout_segment is not None + assert experiment.rollout_segment.is_system_segment is True + + +def test_post__rollout_allocations_exceed_100__returns_400( + admin_client_new: APIClient, + environment: Environment, + multivariate_feature: Feature, + multivariate_options: list[MultivariateFeatureOption], + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + option_a, option_b, _ = multivariate_options + + # When the allocations sum to more than 100% + response = admin_client_new.post( + _list_url(environment), + data={ + "feature": multivariate_feature.id, + "name": "Rollout experiment", + "hypothesis": "It will work", + "experiment_rollout": { + "enabled": True, + "rollout_percentage": 30, + "feature_state_value": {"type": "string", "value": "control"}, + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": option_a.id, + "percentage_allocation": 60, + }, + { + "multivariate_feature_option": option_b.id, + "percentage_allocation": 60, + }, + ], + }, + }, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "exceed" in str(response.json()).lower() + + +def test_post__rollout_mv_option_not_on_feature__returns_400( + admin_client_new: APIClient, + environment: Environment, + multivariate_feature: Feature, + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + + # When + response = admin_client_new.post( + _list_url(environment), + data={ + "feature": multivariate_feature.id, + "name": "Rollout experiment", + "hypothesis": "It will work", + "experiment_rollout": { + "enabled": True, + "rollout_percentage": 30, + "feature_state_value": {"type": "string", "value": "control"}, + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": 999999, + "percentage_allocation": 100, + }, + ], + }, + }, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "do not belong to the feature" in str(response.json()) + + +def test_action_rollout__valid__updates_percentage( + admin_client_new: APIClient, + environment: Environment, + experiment_with_rollout: Experiment, + multivariate_options: list[MultivariateFeatureOption], + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + option_a, option_b, _ = multivariate_options + + # When + response = admin_client_new.patch( + _action_url(environment, experiment_with_rollout, "rollout"), + data={ + "enabled": False, + "rollout_percentage": 75, + "feature_state_value": {"type": "string", "value": "control"}, + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": option_a.id, + "percentage_allocation": 50, + }, + { + "multivariate_feature_option": option_b.id, + "percentage_allocation": 50, + }, + ], + }, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_200_OK + condition = Condition.objects.get( + rule__segment=experiment_with_rollout.rollout_segment + ) + assert condition.value == "75.0" + + +def test_action_rollout__running_experiment__returns_400( + admin_client_new: APIClient, + environment: Environment, + experiment_with_rollout: Experiment, + multivariate_options: list[MultivariateFeatureOption], + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + experiment_with_rollout.status = ExperimentStatus.RUNNING + experiment_with_rollout.save() + option_a, option_b, _ = multivariate_options + + # When + response = admin_client_new.patch( + _action_url(environment, experiment_with_rollout, "rollout"), + data={ + "enabled": True, + "rollout_percentage": 75, + "feature_state_value": {"type": "string", "value": "control"}, + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": option_a.id, + "percentage_allocation": 50, + }, + { + "multivariate_feature_option": option_b.id, + "percentage_allocation": 50, + }, + ], + }, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + + +def test_action_rollout__mv_option_not_on_feature__returns_400( + admin_client_new: APIClient, + environment: Environment, + experiment_with_rollout: Experiment, + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + + # When + response = admin_client_new.patch( + _action_url(environment, experiment_with_rollout, "rollout"), + data={ + "enabled": True, + "rollout_percentage": 75, + "feature_state_value": {"type": "string", "value": "control"}, + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": 999999, + "percentage_allocation": 100, + }, + ], + }, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "do not belong to the feature" in str(response.json()) + + +def test_patch__experiment_rollout_on_update__returns_400( + admin_client_new: APIClient, + environment: Environment, + experiment: Experiment, + enable_features: EnableFeaturesFixture, +) -> None: + # Given + enable_features(EXPERIMENT_FLAG) + + # When + response = admin_client_new.patch( + _detail_url(environment, experiment), + data={ + "experiment_rollout": { + "enabled": True, + "rollout_percentage": 30, + "feature_state_value": {"type": "string", "value": "control"}, + }, + }, + format="json", + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "Cannot change the rollout" in str(response.json()) diff --git a/api/tests/unit/experimentation/test_services.py b/api/tests/unit/experimentation/test_services.py index aa138bc0c232..b0063c9ef247 100644 --- a/api/tests/unit/experimentation/test_services.py +++ b/api/tests/unit/experimentation/test_services.py @@ -2,10 +2,13 @@ from datetime import datetime, timezone import pytest +from flag_engine.segments.constants import PERCENTAGE_SPLIT from pytest_django.fixtures import SettingsWrapper from pytest_mock import MockerFixture from pytest_structlog import StructuredLogCapture +from rest_framework.exceptions import ValidationError +from core.dataclasses import AuthorData from environments.models import Environment from experimentation import services from experimentation.dataclasses import ( @@ -35,6 +38,9 @@ from features.models import Feature, FeatureState from features.multivariate.models import MultivariateFeatureOption from features.value_types import STRING +from features.versioning.dataclasses import MultivariateValueChangeSet +from segments.models import Condition +from users.models import FFAdminUser def test_get_clickhouse_client__configured_url__builds_client_with_timeouts( @@ -1301,3 +1307,148 @@ def test_compute_results_summary__experiment__queries_warehouse_and_builds( assert summary.srm_p_value == pytest.approx(1.0) assert summary.metrics[0].metric_id == metric.id assert summary.metrics[0].inference["variant_a"] is not None + + +def test_create_experiment_rollout__valid__creates_system_segment_and_override( + experiment: Experiment, + multivariate_options: list[MultivariateFeatureOption], + admin_user: FFAdminUser, +) -> None: + # Given + option_a, option_b, _ = multivariate_options + + # When + services.create_experiment_rollout( + experiment, + enabled=True, + rollout_percentage=42.0, + feature_state_value="control", + value_type="string", + multivariate_values=[ + MultivariateValueChangeSet(option_a.id, 60.0), + MultivariateValueChangeSet(option_b.id, 40.0), + ], + author=AuthorData(user=admin_user), + ) + + # Then + experiment.refresh_from_db() + segment = experiment.rollout_segment + assert segment is not None + assert segment.is_system_segment is True + condition = Condition.objects.get(rule__segment=segment) + assert condition.operator == PERCENTAGE_SPLIT + assert condition.value == "42.0" + + override = FeatureState.objects.get( + environment=experiment.environment, + feature=experiment.feature, + feature_segment__segment=segment, + ) + assert override.enabled is True + allocations = { + mv.multivariate_feature_option_id: mv.percentage_allocation + for mv in override.multivariate_feature_state_values.all() + } + assert allocations == {option_a.id: 60.0, option_b.id: 40.0} + + +def test_update_experiment_rollout__valid__updates_percentage_and_enabled( + experiment: Experiment, + multivariate_options: list[MultivariateFeatureOption], + admin_user: FFAdminUser, +) -> None: + # Given + option_a, option_b, _ = multivariate_options + author = AuthorData(user=admin_user) + multivariate_values = [ + MultivariateValueChangeSet(option_a.id, 50.0), + MultivariateValueChangeSet(option_b.id, 50.0), + ] + services.create_experiment_rollout( + experiment, + enabled=True, + rollout_percentage=20.0, + feature_state_value="control", + value_type="string", + multivariate_values=multivariate_values, + author=author, + ) + + # When + services.update_experiment_rollout( + experiment, + enabled=False, + rollout_percentage=80.0, + feature_state_value="control", + value_type="string", + multivariate_values=multivariate_values, + author=author, + ) + + # Then + condition = Condition.objects.get(rule__segment=experiment.rollout_segment) + assert condition.value == "80.0" + override = FeatureState.objects.get( + environment=experiment.environment, + feature=experiment.feature, + feature_segment__segment=experiment.rollout_segment, + ) + assert override.enabled is False + + +@pytest.mark.parametrize( + "status", + [ExperimentStatus.RUNNING, ExperimentStatus.COMPLETED], +) +def test_update_experiment_rollout__running_or_completed__raises( + status: ExperimentStatus, + experiment: Experiment, + multivariate_options: list[MultivariateFeatureOption], + admin_user: FFAdminUser, +) -> None: + # Given + author = AuthorData(user=admin_user) + services.create_experiment_rollout( + experiment, + enabled=True, + rollout_percentage=20.0, + feature_state_value="control", + value_type="string", + multivariate_values=[], + author=author, + ) + experiment.status = status + experiment.save() + + # When / Then + with pytest.raises(ValidationError): + services.update_experiment_rollout( + experiment, + enabled=True, + rollout_percentage=50.0, + feature_state_value="control", + value_type="string", + multivariate_values=[], + author=author, + ) + + +def test_update_experiment_rollout__no_rollout__raises( + experiment: Experiment, + admin_user: FFAdminUser, +) -> None: + # Given + author = AuthorData(user=admin_user) + + # When / Then + with pytest.raises(ValidationError): + services.update_experiment_rollout( + experiment, + enabled=True, + rollout_percentage=50.0, + feature_state_value="control", + value_type="string", + multivariate_values=[], + author=author, + ) diff --git a/api/tests/unit/features/feature_states/test_serializers.py b/api/tests/unit/features/feature_states/test_serializers.py index 471e3110b1ec..42eedd413584 100644 --- a/api/tests/unit/features/feature_states/test_serializers.py +++ b/api/tests/unit/features/feature_states/test_serializers.py @@ -8,6 +8,7 @@ FeatureValueSerializer, UpdateFlagSerializer, UpdateFlagV2Serializer, + validate_multivariate_state_values, ) from features.models import Feature from projects.models import Project @@ -179,3 +180,143 @@ def test_update_flag_serializer__cross_project_segment__returns_invalid( # Then assert is_valid is False assert "not found in project" in str(serializer.errors) + + +def test_update_flag_v2_serializer__mv_option_not_on_feature__returns_invalid( + multivariate_feature: Feature, + environment: Environment, + segment: Segment, +) -> None: + # Given + serializer = UpdateFlagV2Serializer( + data={ + "feature": {"name": multivariate_feature.name}, + "environment_default": { + "enabled": True, + "value": {"type": "string", "value": "default"}, + }, + "segment_overrides": [ + { + "segment_id": segment.id, + "enabled": True, + "value": {"type": "string", "value": "test"}, + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": 999999, + "percentage_allocation": 100, + } + ], + }, + ], + }, + context={"environment": environment}, + ) + + # When + is_valid = serializer.is_valid() + + # Then + assert is_valid is False + assert "do not belong to the feature" in str(serializer.errors) + + +def test_update_flag_v2_serializer__duplicate_mv_option__returns_invalid( + multivariate_feature: Feature, + multivariate_options: list[typing.Any], + environment: Environment, + segment: Segment, +) -> None: + # Given the same multivariate option is passed twice + option = multivariate_options[0] + serializer = UpdateFlagV2Serializer( + data={ + "feature": {"name": multivariate_feature.name}, + "environment_default": { + "enabled": True, + "value": {"type": "string", "value": "default"}, + }, + "segment_overrides": [ + { + "segment_id": segment.id, + "enabled": True, + "value": {"type": "string", "value": "test"}, + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": option.id, + "percentage_allocation": 40, + }, + { + "multivariate_feature_option": option.id, + "percentage_allocation": 60, + }, + ], + }, + ], + }, + context={"environment": environment}, + ) + + # When + is_valid = serializer.is_valid() + + # Then + assert is_valid is False + assert "must be unique" in str(serializer.errors) + + +def test_validate_multivariate_state_values__empty_list__is_noop( + feature: Feature, +) -> None: + # Given + multivariate_values: list[dict[str, typing.Any]] = [] + + # When / Then no exception is raised + validate_multivariate_state_values(feature, multivariate_values) + + +def test_update_flag_v2_serializer__valid_mv_option__change_set_carries_mv( + multivariate_feature: Feature, + multivariate_options: list, # type: ignore[type-arg] + environment: Environment, + segment: Segment, + admin_user: typing.Any, + rf: typing.Any, +) -> None: + # Given + option = multivariate_options[0] + request = rf.post("/") + request.user = admin_user + serializer = UpdateFlagV2Serializer( + data={ + "feature": {"name": multivariate_feature.name}, + "environment_default": { + "enabled": True, + "value": {"type": "string", "value": "default"}, + }, + "segment_overrides": [ + { + "segment_id": segment.id, + "enabled": True, + "value": {"type": "string", "value": "test"}, + "multivariate_feature_state_values": [ + { + "multivariate_feature_option": option.id, + "percentage_allocation": 75, + } + ], + }, + ], + }, + context={"environment": environment, "request": request}, + ) + + # When + is_valid = serializer.is_valid() + change_set = serializer.change_set_v2 + + # Then + assert is_valid is True + mv_values = change_set.segment_overrides[0].multivariate_values + assert mv_values is not None + assert mv_values[0].multivariate_feature_option_id == option.id + assert mv_values[0].percentage_allocation == 75 diff --git a/api/tests/unit/features/versioning/test_unit_versioning_versioning_service.py b/api/tests/unit/features/versioning/test_unit_versioning_versioning_service.py index 77b9c1a6488a..67e9d2e5dbc3 100644 --- a/api/tests/unit/features/versioning/test_unit_versioning_versioning_service.py +++ b/api/tests/unit/features/versioning/test_unit_versioning_versioning_service.py @@ -1,10 +1,13 @@ from datetime import timedelta +import pytest from django.db.models import Q from django.utils import timezone from pytest_django import DjangoAssertNumQueries +from rest_framework.exceptions import ValidationError from core.constants import STRING +from core.dataclasses import AuthorData from environments.identities.models import Identity from environments.models import Environment from features.models import Feature, FeatureSegment, FeatureState @@ -12,12 +15,20 @@ MultivariateFeatureOption, MultivariateFeatureStateValue, ) +from features.versioning.dataclasses import ( + FlagChangeSet, + FlagChangeSetV2, + MultivariateValueChangeSet, + SegmentOverrideChangeSet, +) from features.versioning.models import EnvironmentFeatureVersion from features.versioning.versioning_service import ( get_current_live_environment_feature_version, get_environment_flags_list, get_environment_flags_queryset, get_updated_feature_states_for_version, + update_flag, + update_flag_v2, ) from projects.models import Project from segments.models import Segment @@ -621,3 +632,298 @@ def test_get_environment_flags_list__from_replica__returns_feature_states( # Then assert len(result) >= 1 assert result[0].feature == feature + + +def _mv_change_set( + author: AuthorData, + segment: Segment, + *, + multivariate_values: list[MultivariateValueChangeSet] | None, +) -> FlagChangeSetV2: + return FlagChangeSetV2( + author=author, + environment_default_enabled=True, + environment_default_value="control", + environment_default_type="string", + segment_overrides=[ + SegmentOverrideChangeSet( + segment_id=segment.id, + enabled=True, + feature_state_value="control", + type_="string", + multivariate_values=multivariate_values, + ) + ], + ) + + +def _get_live_override( + environment: Environment, feature: Feature, segment: Segment +) -> FeatureState: + if environment.use_v2_feature_versioning: + version = get_current_live_environment_feature_version( + environment_id=environment.id, feature_id=feature.id + ) + assert version is not None + override: FeatureState = version.feature_states.get( + feature_segment__segment=segment + ) + return override + override = FeatureState.objects.get( + environment=environment, feature=feature, feature_segment__segment=segment + ) + return override + + +def _override_allocations(override: FeatureState) -> dict[int, float]: + return { + mv.multivariate_feature_option_id: mv.percentage_allocation + for mv in override.multivariate_feature_state_values.all() + } + + +@pytest.mark.parametrize( + "environment_fixture_name", + ["environment", "environment_v2_versioning"], +) +def test_update_flag_v2__new_segment_override_with_mv__creates_mv_values( + environment_fixture_name: str, + multivariate_feature: Feature, + multivariate_options: list[MultivariateFeatureOption], + segment: Segment, + admin_user: FFAdminUser, + request: pytest.FixtureRequest, +) -> None: + # Given + environment: Environment = request.getfixturevalue(environment_fixture_name) + option_a, option_b, _ = multivariate_options + change_set = _mv_change_set( + AuthorData(user=admin_user), + segment, + multivariate_values=[ + MultivariateValueChangeSet(option_a.id, 60.0), + MultivariateValueChangeSet(option_b.id, 40.0), + ], + ) + + # When + update_flag_v2(environment, multivariate_feature, change_set) + + # Then + override = _get_live_override(environment, multivariate_feature, segment) + assert _override_allocations(override) == {option_a.id: 60.0, option_b.id: 40.0} + + +@pytest.mark.parametrize( + "environment_fixture_name", + ["environment", "environment_v2_versioning"], +) +def test_update_flag_v2__existing_override_mv_changed__updates_allocations( + environment_fixture_name: str, + multivariate_feature: Feature, + multivariate_options: list[MultivariateFeatureOption], + segment: Segment, + admin_user: FFAdminUser, + request: pytest.FixtureRequest, +) -> None: + # Given + environment: Environment = request.getfixturevalue(environment_fixture_name) + author = AuthorData(user=admin_user) + option_a, option_b, _ = multivariate_options + update_flag_v2( + environment, + multivariate_feature, + _mv_change_set( + author, + segment, + multivariate_values=[ + MultivariateValueChangeSet(option_a.id, 60.0), + MultivariateValueChangeSet(option_b.id, 40.0), + ], + ), + ) + + # When + update_flag_v2( + environment, + multivariate_feature, + _mv_change_set( + author, + segment, + multivariate_values=[ + MultivariateValueChangeSet(option_a.id, 70.0), + MultivariateValueChangeSet(option_b.id, 30.0), + ], + ), + ) + + # Then + override = _get_live_override(environment, multivariate_feature, segment) + assert _override_allocations(override) == {option_a.id: 70.0, option_b.id: 30.0} + + +@pytest.mark.parametrize( + "environment_fixture_name", + ["environment", "environment_v2_versioning"], +) +def test_update_flag_v2__option_not_passed__is_retained( + environment_fixture_name: str, + multivariate_feature: Feature, + multivariate_options: list[MultivariateFeatureOption], + segment: Segment, + admin_user: FFAdminUser, + request: pytest.FixtureRequest, +) -> None: + # Given + environment: Environment = request.getfixturevalue(environment_fixture_name) + author = AuthorData(user=admin_user) + option_a, option_b, _ = multivariate_options + update_flag_v2( + environment, + multivariate_feature, + _mv_change_set( + author, + segment, + multivariate_values=[ + MultivariateValueChangeSet(option_a.id, 50.0), + MultivariateValueChangeSet(option_b.id, 50.0), + ], + ), + ) + + # When only option_a is passed + update_flag_v2( + environment, + multivariate_feature, + _mv_change_set( + author, + segment, + multivariate_values=[MultivariateValueChangeSet(option_a.id, 30.0)], + ), + ) + + # Then option_b is left untouched + override = _get_live_override(environment, multivariate_feature, segment) + assert _override_allocations(override) == {option_a.id: 30.0, option_b.id: 50.0} + + +@pytest.mark.parametrize( + "environment_fixture_name", + ["environment", "environment_v2_versioning"], +) +def test_update_flag_v2__no_mv_values__leaves_existing_mv_untouched( + environment_fixture_name: str, + multivariate_feature: Feature, + multivariate_options: list[MultivariateFeatureOption], + segment: Segment, + admin_user: FFAdminUser, + request: pytest.FixtureRequest, +) -> None: + # Given + environment: Environment = request.getfixturevalue(environment_fixture_name) + author = AuthorData(user=admin_user) + option_a, option_b, _ = multivariate_options + update_flag_v2( + environment, + multivariate_feature, + _mv_change_set( + author, + segment, + multivariate_values=[ + MultivariateValueChangeSet(option_a.id, 60.0), + MultivariateValueChangeSet(option_b.id, 40.0), + ], + ), + ) + + # When + update_flag_v2( + environment, + multivariate_feature, + _mv_change_set(author, segment, multivariate_values=None), + ) + + # Then + override = _get_live_override(environment, multivariate_feature, segment) + assert _override_allocations(override) == {option_a.id: 60.0, option_b.id: 40.0} + + +@pytest.mark.parametrize( + "environment_fixture_name", + ["environment", "environment_v2_versioning"], +) +def test_update_flag__segment_override_with_mv__sets_mv_values( + environment_fixture_name: str, + multivariate_feature: Feature, + multivariate_options: list[MultivariateFeatureOption], + segment: Segment, + admin_user: FFAdminUser, + request: pytest.FixtureRequest, +) -> None: + # Given + environment: Environment = request.getfixturevalue(environment_fixture_name) + option_a, option_b, _ = multivariate_options + + # When + update_flag( + environment, + multivariate_feature, + FlagChangeSet( + author=AuthorData(user=admin_user), + enabled=True, + feature_state_value="control", + type_="string", + segment_id=segment.id, + multivariate_values=[ + MultivariateValueChangeSet(option_a.id, 70.0), + MultivariateValueChangeSet(option_b.id, 30.0), + ], + ), + ) + + # Then + override = _get_live_override(environment, multivariate_feature, segment) + assert _override_allocations(override) == {option_a.id: 70.0, option_b.id: 30.0} + + +@pytest.mark.parametrize( + "environment_fixture_name", + ["environment", "environment_v2_versioning"], +) +def test_update_flag_v2__retained_plus_passed_exceeds_100__raises( + environment_fixture_name: str, + multivariate_feature: Feature, + multivariate_options: list[MultivariateFeatureOption], + segment: Segment, + admin_user: FFAdminUser, + request: pytest.FixtureRequest, +) -> None: + # Given an override allocating option_a 80% and option_b 20% + environment: Environment = request.getfixturevalue(environment_fixture_name) + author = AuthorData(user=admin_user) + option_a, option_b, _ = multivariate_options + update_flag_v2( + environment, + multivariate_feature, + _mv_change_set( + author, + segment, + multivariate_values=[ + MultivariateValueChangeSet(option_a.id, 80.0), + MultivariateValueChangeSet(option_b.id, 20.0), + ], + ), + ) + + # When option_b alone is raised to 100% (retained option_a 80% → 180% total) + # Then it is rejected + with pytest.raises(ValidationError): + update_flag_v2( + environment, + multivariate_feature, + _mv_change_set( + author, + segment, + multivariate_values=[MultivariateValueChangeSet(option_b.id, 100.0)], + ), + ) diff --git a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md index 645eb0decb76..9c496e0f9304 100644 --- a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md +++ b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md @@ -476,7 +476,7 @@ Attributes: ### `warehouse.connection.connected` Logged at `info` from: - - `api/experimentation/services.py:545` + - `api/experimentation/services.py:632` Attributes: - `environment.id` @@ -485,7 +485,7 @@ Attributes: ### `warehouse.connection.test_event_sent` Logged at `info` from: - - `api/experimentation/services.py:525` + - `api/experimentation/services.py:612` Attributes: - `environment.id` @@ -494,7 +494,7 @@ Attributes: ### `warehouse.srm.overallocated` Logged at `error` from: - - `api/experimentation/services.py:381` + - `api/experimentation/services.py:388` Attributes: - `environment.id` @@ -504,7 +504,7 @@ Attributes: ### `warehouse.srm.unkeyed_variant` Logged at `error` from: - - `api/experimentation/services.py:367` + - `api/experimentation/services.py:374` Attributes: - `environment.id` From 14b16f09dd7a81baa311ee3b4f71d3638e70c075 Mon Sep 17 00:00:00 2001 From: Gagan Trivedi Date: Wed, 24 Jun 2026 15:24:43 +0530 Subject: [PATCH 2/5] fix(experimentation): wrap rollout update in a transaction 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. --- api/experimentation/services.py | 34 +++++++++++-------- .../unit/experimentation/test_services.py | 31 +++++++++++++++++ .../observability/_events-catalogue.md | 8 ++--- 3 files changed, 54 insertions(+), 19 deletions(-) diff --git a/api/experimentation/services.py b/api/experimentation/services.py index fb23b88b54b0..dbf4f81a4c6e 100644 --- a/api/experimentation/services.py +++ b/api/experimentation/services.py @@ -8,6 +8,7 @@ from clickhouse_driver import Client from clickhouse_driver.util.helpers import parse_url from django.conf import settings +from django.db import transaction from django.db.models import Q from django.utils import timezone from flag_engine.segments.constants import PERCENTAGE_SPLIT @@ -582,21 +583,24 @@ def update_experiment_rollout( if segment is None: raise ValidationError("Experiment has no rollout to update.") - condition = Condition.objects.get(rule__segment=segment, operator=PERCENTAGE_SPLIT) - condition.value = str(rollout_percentage) - condition.save() - update_flag( - experiment.environment, - experiment.feature, - FlagChangeSet( - author=author, - enabled=enabled, - feature_state_value=feature_state_value, - type_=value_type, - segment_id=segment.id, - multivariate_values=multivariate_values, - ), - ) + with transaction.atomic(): + condition = Condition.objects.get( + rule__segment=segment, operator=PERCENTAGE_SPLIT + ) + condition.value = str(rollout_percentage) + condition.save() + update_flag( + experiment.environment, + experiment.feature, + FlagChangeSet( + author=author, + enabled=enabled, + feature_state_value=feature_state_value, + type_=value_type, + segment_id=segment.id, + multivariate_values=multivariate_values, + ), + ) def mark_warehouse_pending_connection( diff --git a/api/tests/unit/experimentation/test_services.py b/api/tests/unit/experimentation/test_services.py index b0063c9ef247..b69f8c468bbf 100644 --- a/api/tests/unit/experimentation/test_services.py +++ b/api/tests/unit/experimentation/test_services.py @@ -1452,3 +1452,34 @@ def test_update_experiment_rollout__no_rollout__raises( multivariate_values=[], author=author, ) + + +def test_update_experiment_rollout__update_flag_fails__rolls_back( + experiment_with_rollout: Experiment, + admin_user: FFAdminUser, + mocker: MockerFixture, +) -> None: + # Given a rollout at 20% and update_flag will fail mid-update + experiment = experiment_with_rollout + mocker.patch( + "experimentation.services.update_flag", + side_effect=RuntimeError("boom"), + ) + + # When the update fails + with pytest.raises(RuntimeError): + services.update_experiment_rollout( + experiment, + enabled=False, + rollout_percentage=80.0, + feature_state_value="control", + value_type="string", + multivariate_values=[], + author=AuthorData(user=admin_user), + ) + + # Then the percentage split change is rolled back + condition = Condition.objects.get( + rule__segment=experiment.rollout_segment, operator=PERCENTAGE_SPLIT + ) + assert condition.value == "20.0" diff --git a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md index 9c496e0f9304..3f54a80b7451 100644 --- a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md +++ b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md @@ -476,7 +476,7 @@ Attributes: ### `warehouse.connection.connected` Logged at `info` from: - - `api/experimentation/services.py:632` + - `api/experimentation/services.py:636` Attributes: - `environment.id` @@ -485,7 +485,7 @@ Attributes: ### `warehouse.connection.test_event_sent` Logged at `info` from: - - `api/experimentation/services.py:612` + - `api/experimentation/services.py:616` Attributes: - `environment.id` @@ -494,7 +494,7 @@ Attributes: ### `warehouse.srm.overallocated` Logged at `error` from: - - `api/experimentation/services.py:388` + - `api/experimentation/services.py:389` Attributes: - `environment.id` @@ -504,7 +504,7 @@ Attributes: ### `warehouse.srm.unkeyed_variant` Logged at `error` from: - - `api/experimentation/services.py:374` + - `api/experimentation/services.py:375` Attributes: - `environment.id` From b7481ea4c3e1c4f814647bdb9762afc188b9a3f0 Mon Sep 17 00:00:00 2001 From: Gagan Trivedi Date: Wed, 24 Jun 2026 15:52:58 +0530 Subject: [PATCH 3/5] refactor(experimentation): model the rollout as a RolloutSpec applied 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. --- api/experimentation/dataclasses.py | 13 ++ api/experimentation/serializers.py | 37 ++--- api/experimentation/services.py | 91 +++++------ api/experimentation/views.py | 13 +- api/tests/unit/experimentation/conftest.py | 25 +-- .../unit/experimentation/test_services.py | 143 +++++++++--------- .../observability/_events-catalogue.md | 8 +- 7 files changed, 157 insertions(+), 173 deletions(-) diff --git a/api/experimentation/dataclasses.py b/api/experimentation/dataclasses.py index 8b6fdc557400..d8db1624ed34 100644 --- a/api/experimentation/dataclasses.py +++ b/api/experimentation/dataclasses.py @@ -1,8 +1,21 @@ from dataclasses import dataclass from datetime import datetime +from core.dataclasses import AuthorData from experimentation.stats import Inference, VariantStats from experimentation.types import ExposureGranularity +from features.feature_states.models import FeatureValueType +from features.versioning.dataclasses import MultivariateValueChangeSet + + +@dataclass(frozen=True) +class RolloutSpec: + enabled: bool + rollout_percentage: float + feature_state_value: str + value_type: FeatureValueType + multivariate_values: list[MultivariateValueChangeSet] + author: AuthorData @dataclass(frozen=True) diff --git a/api/experimentation/serializers.py b/api/experimentation/serializers.py index 89642c9fb1b9..a5d5e97d702d 100644 --- a/api/experimentation/serializers.py +++ b/api/experimentation/serializers.py @@ -6,7 +6,7 @@ from core.dataclasses import AuthorData from environments.models import Environment -from experimentation.dataclasses import WarehouseEventStats +from experimentation.dataclasses import RolloutSpec, WarehouseEventStats from experimentation.metric_definitions import validate_metric_definition from experimentation.models import ( ExpectedDirection, @@ -19,7 +19,7 @@ WarehouseConnection, WarehouseType, ) -from experimentation.services import create_experiment_rollout +from experimentation.services import apply_experiment_rollout from experimentation.types import ( SNOWFLAKE_DEFAULTS, MetricExperimentResult, @@ -28,7 +28,6 @@ from features.feature_states.serializers import ( FeatureValueSerializer, MultivariateValueSerializer, - validate_multivariate_state_values, ) from features.feature_types import MULTIVARIATE from features.models import Feature @@ -226,22 +225,22 @@ class ExperimentRolloutSerializer(serializers.Serializer): # type: ignore[type- ) @staticmethod - def to_service_kwargs(data: dict[str, Any], request: Any) -> dict[str, Any]: + def to_spec(data: dict[str, Any], request: Any) -> RolloutSpec: value = data["feature_state_value"] - return { - "enabled": data["enabled"], - "rollout_percentage": data["rollout_percentage"], - "feature_state_value": value["value"], - "value_type": value["type"], - "multivariate_values": [ + return RolloutSpec( + enabled=data["enabled"], + rollout_percentage=data["rollout_percentage"], + feature_state_value=value["value"], + value_type=value["type"], + multivariate_values=[ MultivariateValueChangeSet( multivariate_feature_option_id=mv["multivariate_feature_option"], percentage_allocation=mv["percentage_allocation"], ) for mv in data.get("multivariate_feature_state_values", []) ], - "author": AuthorData.from_request(request), - } + author=AuthorData.from_request(request), + ) class ExperimentSerializer(serializers.ModelSerializer): # type: ignore[type-arg] @@ -309,18 +308,8 @@ def validate(self, attrs: dict[str, Any]) -> dict[str, Any]: } ) self._validate_metrics(attrs.get("metrics") or []) - self._validate_rollout(attrs) return attrs - def _validate_rollout(self, attrs: dict[str, Any]) -> None: - rollout = attrs.get("experiment_rollout") - feature = attrs.get("feature") - if not rollout or feature is None: - return - validate_multivariate_state_values( - feature, rollout.get("multivariate_feature_state_values", []) - ) - def _validate_metrics(self, metrics: list[dict[str, Any]]) -> None: metric_ids = [entry["metric"].id for entry in metrics] if len(metric_ids) != len(set(metric_ids)): @@ -342,9 +331,9 @@ def create(self, validated_data: dict[str, Any]) -> Experiment: for entry in metrics ) if rollout is not None: - create_experiment_rollout( + apply_experiment_rollout( experiment, - **ExperimentRolloutSerializer.to_service_kwargs( + ExperimentRolloutSerializer.to_spec( rollout, self.context["request"] ), ) diff --git a/api/experimentation/services.py b/api/experimentation/services.py index dbf4f81a4c6e..aa622fbc9e16 100644 --- a/api/experimentation/services.py +++ b/api/experimentation/services.py @@ -16,7 +16,6 @@ from audit.models import AuditLog from audit.related_object_type import RelatedObjectType -from core.dataclasses import AuthorData from experimentation.constants import ( CONTROL_VARIANT_KEY, EXPERIMENT_FLAG, @@ -36,6 +35,7 @@ MetricSpec, ResultsAggregates, ResultsSummary, + RolloutSpec, WarehouseEventStats, ) from experimentation.models import ( @@ -54,7 +54,7 @@ srm_p_value, ) from features.models import FeatureState -from features.versioning.dataclasses import FlagChangeSet, MultivariateValueChangeSet +from features.versioning.dataclasses import FlagChangeSet from features.versioning.versioning_service import update_flag from integrations.flagsmith.client import get_openfeature_client from segments.models import Condition, Segment, SegmentRule @@ -65,7 +65,6 @@ from experimentation.models import Experiment, Metric, WarehouseConnection from experimentation.types import ExposureGranularity - from features.feature_states.models import FeatureValueType from organisations.models import Organisation from users.models import FFAdminUser @@ -538,67 +537,59 @@ def _create_rollout_segment( return segment -def create_experiment_rollout( - experiment: Experiment, - *, - enabled: bool, - rollout_percentage: float, - feature_state_value: str, - value_type: FeatureValueType, - multivariate_values: list[MultivariateValueChangeSet], - author: AuthorData, -) -> None: - segment = _create_rollout_segment(experiment, rollout_percentage) - experiment.rollout_segment = segment - experiment.save() - update_flag( - experiment.environment, - experiment.feature, - FlagChangeSet( - author=author, - enabled=enabled, - feature_state_value=feature_state_value, - type_=value_type, - segment_id=segment.id, - multivariate_values=multivariate_values, - ), +def validate_rollout_spec(experiment: Experiment, spec: RolloutSpec) -> None: + option_ids = [v.multivariate_feature_option_id for v in spec.multivariate_values] + if len(option_ids) != len(set(option_ids)): + raise ValidationError("Multivariate options must be unique") + valid_option_ids = set( + experiment.feature.multivariate_options.values_list("id", flat=True) ) - - -def update_experiment_rollout( - experiment: Experiment, - *, - enabled: bool, - rollout_percentage: float, - feature_state_value: str, - value_type: FeatureValueType, - multivariate_values: list[MultivariateValueChangeSet], - author: AuthorData, -) -> None: - if experiment.status in (ExperimentStatus.RUNNING, ExperimentStatus.COMPLETED): + if invalid := set(option_ids) - valid_option_ids: raise ValidationError( - f"Cannot update the rollout of a {experiment.status} experiment." + f"Multivariate options {sorted(invalid)} do not belong to the feature" + ) + total = sum(v.percentage_allocation for v in spec.multivariate_values) + if total > 100: + raise ValidationError( + f"Multivariate allocations must not exceed 100%, got {total}%." ) - segment = experiment.rollout_segment - if segment is None: - raise ValidationError("Experiment has no rollout to update.") - with transaction.atomic(): + +def _sync_rollout_segment( + experiment: Experiment, rollout_percentage: float +) -> Segment: + segment = experiment.rollout_segment + if segment is not None: condition = Condition.objects.get( rule__segment=segment, operator=PERCENTAGE_SPLIT ) condition.value = str(rollout_percentage) condition.save() + return segment + segment = _create_rollout_segment(experiment, rollout_percentage) + experiment.rollout_segment = segment + experiment.save() + return segment + + +def apply_experiment_rollout(experiment: Experiment, spec: RolloutSpec) -> None: + if experiment.status in (ExperimentStatus.RUNNING, ExperimentStatus.COMPLETED): + raise ValidationError( + f"Cannot change the rollout of a {experiment.status} experiment." + ) + validate_rollout_spec(experiment, spec) + with transaction.atomic(): + segment = _sync_rollout_segment(experiment, spec.rollout_percentage) update_flag( experiment.environment, experiment.feature, FlagChangeSet( - author=author, - enabled=enabled, - feature_state_value=feature_state_value, - type_=value_type, + author=spec.author, + enabled=spec.enabled, + feature_state_value=spec.feature_state_value, + type_=spec.value_type, segment_id=segment.id, - multivariate_values=multivariate_values, + multivariate_values=spec.multivariate_values, ), ) diff --git a/api/experimentation/views.py b/api/experimentation/views.py index d84192eec844..90689ff469ab 100644 --- a/api/experimentation/views.py +++ b/api/experimentation/views.py @@ -49,21 +49,18 @@ ) from experimentation.services import ( annotate_warehouse_event_stats, + apply_experiment_rollout, create_experiment_audit_log, create_metric_audit_log, create_warehouse_audit_log, mark_warehouse_pending_connection, refresh_warehouse_connection_status, transition_experiment_status, - update_experiment_rollout, ) from experimentation.tasks import ( compute_experiment_exposures, compute_experiment_results, ) -from features.feature_states.serializers import ( - validate_multivariate_state_values, -) from users.models import FFAdminUser logger = logging.getLogger(__name__) @@ -300,13 +297,9 @@ def rollout(self, request: Request, **kwargs: object) -> Response: experiment: Experiment = self.get_object() serializer = ExperimentRolloutSerializer(data=request.data) serializer.is_valid(raise_exception=True) - data = serializer.validated_data - validate_multivariate_state_values( - experiment.feature, data.get("multivariate_feature_state_values", []) - ) - update_experiment_rollout( + apply_experiment_rollout( experiment, - **ExperimentRolloutSerializer.to_service_kwargs(data, request), + ExperimentRolloutSerializer.to_spec(serializer.validated_data, request), ) return Response(self.get_serializer(experiment).data) diff --git a/api/tests/unit/experimentation/conftest.py b/api/tests/unit/experimentation/conftest.py index 5680ebc17437..ad9c74cce4b7 100644 --- a/api/tests/unit/experimentation/conftest.py +++ b/api/tests/unit/experimentation/conftest.py @@ -5,6 +5,7 @@ from core.dataclasses import AuthorData from environments.models import Environment from experimentation import ingestion_sync_service +from experimentation.dataclasses import RolloutSpec from experimentation.models import ( Experiment, ExperimentStatus, @@ -12,7 +13,7 @@ WarehouseConnection, WarehouseType, ) -from experimentation.services import create_experiment_rollout +from experimentation.services import apply_experiment_rollout from features.models import Feature from features.multivariate.models import MultivariateFeatureOption from features.versioning.dataclasses import MultivariateValueChangeSet @@ -75,16 +76,18 @@ def experiment_with_rollout( admin_user: FFAdminUser, ) -> Experiment: option_a, option_b, _ = multivariate_options - create_experiment_rollout( + apply_experiment_rollout( experiment, - enabled=True, - rollout_percentage=20.0, - feature_state_value="control", - value_type="string", - multivariate_values=[ - MultivariateValueChangeSet(option_a.id, 50.0), - MultivariateValueChangeSet(option_b.id, 50.0), - ], - author=AuthorData(user=admin_user), + RolloutSpec( + enabled=True, + rollout_percentage=20.0, + feature_state_value="control", + value_type="string", + multivariate_values=[ + MultivariateValueChangeSet(option_a.id, 50.0), + MultivariateValueChangeSet(option_b.id, 50.0), + ], + author=AuthorData(user=admin_user), + ), ) return experiment diff --git a/api/tests/unit/experimentation/test_services.py b/api/tests/unit/experimentation/test_services.py index b69f8c468bbf..4c25cbe77f12 100644 --- a/api/tests/unit/experimentation/test_services.py +++ b/api/tests/unit/experimentation/test_services.py @@ -18,6 +18,7 @@ ExposuresTimeseriesPoint, MetricSpec, ResultsAggregates, + RolloutSpec, WarehouseEventStats, ) from experimentation.models import ( @@ -1309,7 +1310,7 @@ def test_compute_results_summary__experiment__queries_warehouse_and_builds( assert summary.metrics[0].inference["variant_a"] is not None -def test_create_experiment_rollout__valid__creates_system_segment_and_override( +def test_apply_experiment_rollout__no_segment__creates_segment_and_override( experiment: Experiment, multivariate_options: list[MultivariateFeatureOption], admin_user: FFAdminUser, @@ -1318,17 +1319,19 @@ def test_create_experiment_rollout__valid__creates_system_segment_and_override( option_a, option_b, _ = multivariate_options # When - services.create_experiment_rollout( + services.apply_experiment_rollout( experiment, - enabled=True, - rollout_percentage=42.0, - feature_state_value="control", - value_type="string", - multivariate_values=[ - MultivariateValueChangeSet(option_a.id, 60.0), - MultivariateValueChangeSet(option_b.id, 40.0), - ], - author=AuthorData(user=admin_user), + RolloutSpec( + enabled=True, + rollout_percentage=42.0, + feature_state_value="control", + value_type="string", + multivariate_values=[ + MultivariateValueChangeSet(option_a.id, 60.0), + MultivariateValueChangeSet(option_b.id, 40.0), + ], + author=AuthorData(user=admin_user), + ), ) # Then @@ -1353,37 +1356,29 @@ def test_create_experiment_rollout__valid__creates_system_segment_and_override( assert allocations == {option_a.id: 60.0, option_b.id: 40.0} -def test_update_experiment_rollout__valid__updates_percentage_and_enabled( - experiment: Experiment, +def test_apply_experiment_rollout__existing_segment__updates_percentage_and_enabled( + experiment_with_rollout: Experiment, multivariate_options: list[MultivariateFeatureOption], admin_user: FFAdminUser, ) -> None: # Given + experiment = experiment_with_rollout option_a, option_b, _ = multivariate_options - author = AuthorData(user=admin_user) - multivariate_values = [ - MultivariateValueChangeSet(option_a.id, 50.0), - MultivariateValueChangeSet(option_b.id, 50.0), - ] - services.create_experiment_rollout( - experiment, - enabled=True, - rollout_percentage=20.0, - feature_state_value="control", - value_type="string", - multivariate_values=multivariate_values, - author=author, - ) # When - services.update_experiment_rollout( + services.apply_experiment_rollout( experiment, - enabled=False, - rollout_percentage=80.0, - feature_state_value="control", - value_type="string", - multivariate_values=multivariate_values, - author=author, + RolloutSpec( + enabled=False, + rollout_percentage=80.0, + feature_state_value="control", + value_type="string", + multivariate_values=[ + MultivariateValueChangeSet(option_a.id, 50.0), + MultivariateValueChangeSet(option_b.id, 50.0), + ], + author=AuthorData(user=admin_user), + ), ) # Then @@ -1401,84 +1396,84 @@ def test_update_experiment_rollout__valid__updates_percentage_and_enabled( "status", [ExperimentStatus.RUNNING, ExperimentStatus.COMPLETED], ) -def test_update_experiment_rollout__running_or_completed__raises( +def test_apply_experiment_rollout__running_or_completed__raises( status: ExperimentStatus, - experiment: Experiment, - multivariate_options: list[MultivariateFeatureOption], + experiment_with_rollout: Experiment, admin_user: FFAdminUser, ) -> None: # Given - author = AuthorData(user=admin_user) - services.create_experiment_rollout( - experiment, - enabled=True, - rollout_percentage=20.0, - feature_state_value="control", - value_type="string", - multivariate_values=[], - author=author, - ) + experiment = experiment_with_rollout experiment.status = status experiment.save() # When / Then with pytest.raises(ValidationError): - services.update_experiment_rollout( + services.apply_experiment_rollout( experiment, - enabled=True, - rollout_percentage=50.0, - feature_state_value="control", - value_type="string", - multivariate_values=[], - author=author, + RolloutSpec( + enabled=True, + rollout_percentage=50.0, + feature_state_value="control", + value_type="string", + multivariate_values=[], + author=AuthorData(user=admin_user), + ), ) -def test_update_experiment_rollout__no_rollout__raises( +def test_apply_experiment_rollout__duplicate_options__raises( experiment: Experiment, + multivariate_options: list[MultivariateFeatureOption], admin_user: FFAdminUser, ) -> None: # Given - author = AuthorData(user=admin_user) + option_a, _, _ = multivariate_options # When / Then with pytest.raises(ValidationError): - services.update_experiment_rollout( + services.apply_experiment_rollout( experiment, - enabled=True, - rollout_percentage=50.0, - feature_state_value="control", - value_type="string", - multivariate_values=[], - author=author, + RolloutSpec( + enabled=True, + rollout_percentage=20.0, + feature_state_value="control", + value_type="string", + multivariate_values=[ + MultivariateValueChangeSet(option_a.id, 40.0), + MultivariateValueChangeSet(option_a.id, 60.0), + ], + author=AuthorData(user=admin_user), + ), ) -def test_update_experiment_rollout__update_flag_fails__rolls_back( +def test_apply_experiment_rollout__update_flag_fails__rolls_back( experiment_with_rollout: Experiment, admin_user: FFAdminUser, mocker: MockerFixture, ) -> None: - # Given a rollout at 20% and update_flag will fail mid-update + # Given experiment = experiment_with_rollout mocker.patch( "experimentation.services.update_flag", side_effect=RuntimeError("boom"), ) - # When the update fails + # When / Then with pytest.raises(RuntimeError): - services.update_experiment_rollout( + services.apply_experiment_rollout( experiment, - enabled=False, - rollout_percentage=80.0, - feature_state_value="control", - value_type="string", - multivariate_values=[], - author=AuthorData(user=admin_user), + RolloutSpec( + enabled=False, + rollout_percentage=80.0, + feature_state_value="control", + value_type="string", + multivariate_values=[], + author=AuthorData(user=admin_user), + ), ) - # Then the percentage split change is rolled back + # Then condition = Condition.objects.get( rule__segment=experiment.rollout_segment, operator=PERCENTAGE_SPLIT ) diff --git a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md index 3f54a80b7451..729786b2f8a6 100644 --- a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md +++ b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md @@ -476,7 +476,7 @@ Attributes: ### `warehouse.connection.connected` Logged at `info` from: - - `api/experimentation/services.py:636` + - `api/experimentation/services.py:627` Attributes: - `environment.id` @@ -485,7 +485,7 @@ Attributes: ### `warehouse.connection.test_event_sent` Logged at `info` from: - - `api/experimentation/services.py:616` + - `api/experimentation/services.py:607` Attributes: - `environment.id` @@ -494,7 +494,7 @@ Attributes: ### `warehouse.srm.overallocated` Logged at `error` from: - - `api/experimentation/services.py:389` + - `api/experimentation/services.py:388` Attributes: - `environment.id` @@ -504,7 +504,7 @@ Attributes: ### `warehouse.srm.unkeyed_variant` Logged at `error` from: - - `api/experimentation/services.py:375` + - `api/experimentation/services.py:374` Attributes: - `environment.id` From b0f89967e4bcc4f6a1e2e0b043f8ca9a04cd5160 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 24 Jun 2026 10:25:26 +0000 Subject: [PATCH 4/5] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- api/experimentation/services.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/api/experimentation/services.py b/api/experimentation/services.py index aa622fbc9e16..91c10232c2bb 100644 --- a/api/experimentation/services.py +++ b/api/experimentation/services.py @@ -555,9 +555,7 @@ def validate_rollout_spec(experiment: Experiment, spec: RolloutSpec) -> None: ) -def _sync_rollout_segment( - experiment: Experiment, rollout_percentage: float -) -> Segment: +def _sync_rollout_segment(experiment: Experiment, rollout_percentage: float) -> Segment: segment = experiment.rollout_segment if segment is not None: condition = Condition.objects.get( From 49a43d6b5704b361768957f3023385bfc2cad95e Mon Sep 17 00:00:00 2001 From: Gagan Trivedi Date: Wed, 24 Jun 2026 16:10:41 +0530 Subject: [PATCH 5/5] docs: regenerate events catalogue after rollout service reformat --- .../observability/_events-catalogue.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md index 6b663038ca25..e07ceb603f35 100644 --- a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md +++ b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md @@ -476,7 +476,7 @@ Attributes: ### `warehouse.connection.connected` Logged at `info` from: - - `api/experimentation/services.py:627` + - `api/experimentation/services.py:625` Attributes: - `environment.id` @@ -485,7 +485,7 @@ Attributes: ### `warehouse.connection.test_event_sent` Logged at `info` from: - - `api/experimentation/services.py:607` + - `api/experimentation/services.py:605` Attributes: - `environment.id`