Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions api/experimentation/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ class ExperimentRolloutSerializer(serializers.Serializer): # type: ignore[type-
)

@staticmethod
def to_spec(data: dict[str, Any], request: Any) -> RolloutSpec:
def to_spec(data: dict[str, Any], author: AuthorData) -> RolloutSpec:
value = data["feature_state_value"]
return RolloutSpec(
enabled=data["enabled"],
Expand All @@ -242,7 +242,7 @@ def to_spec(data: dict[str, Any], request: Any) -> RolloutSpec:
)
for mv in data.get("multivariate_feature_state_values", [])
],
author=AuthorData.from_request(request),
author=author,
)


Expand Down Expand Up @@ -339,7 +339,7 @@ def create(self, validated_data: dict[str, Any]) -> Experiment:
apply_experiment_rollout(
experiment,
ExperimentRolloutSerializer.to_spec(
rollout, self.context["request"]
rollout, AuthorData.from_request(self.context["request"])
),
)
return experiment
Expand Down
54 changes: 47 additions & 7 deletions api/experimentation/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

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,
Expand Down Expand Up @@ -510,18 +511,29 @@ def transition_experiment_status(
f"Cannot transition from '{experiment.status}' to '{target_status}'."
)

experiment.status = target_status
with transaction.atomic():
experiment.status = target_status

if target_status == ExperimentStatus.RUNNING and not experiment.started_at:
experiment.started_at = timezone.now()
elif target_status == ExperimentStatus.COMPLETED:
experiment.ended_at = timezone.now()
if target_status == ExperimentStatus.RUNNING and not experiment.started_at:
experiment.started_at = timezone.now()
elif target_status == ExperimentStatus.COMPLETED:
experiment.ended_at = timezone.now()

experiment.save()
create_experiment_audit_log(experiment, user, action=target_status)
experiment.save()

if target_status == ExperimentStatus.RUNNING:
enable_experiment_rollout(experiment, _author_from_user(user))

create_experiment_audit_log(experiment, user, action=target_status)
return experiment


def _author_from_user(user: FFAdminUser) -> AuthorData:
if getattr(user, "is_master_api_key_user", False):
return AuthorData(api_key=user.key)
return AuthorData(user=user)


def _create_rollout_segment(
experiment: Experiment, rollout_percentage: float
) -> Segment:
Expand Down Expand Up @@ -631,6 +643,34 @@ def get_experiment_rollout(experiment: Experiment) -> dict[str, typing.Any] | No
}


def enable_experiment_rollout(experiment: Experiment, author: AuthorData) -> None:
"""Enable the rollout segment override so a started experiment serves its
split to traffic. No-op when the experiment has no rollout, or the override
is already enabled."""
# Imported here to avoid a circular import: serializers import this module.
from experimentation.serializers import ExperimentRolloutSerializer

rollout = get_experiment_rollout(experiment)
if rollout is None or rollout["enabled"]:
return

# Re-apply the existing override as-is but enabled, going through update_flag
# so the change is versioned like any other flag write. Multivariate
# allocations are preserved by update_flag when omitted from the change set.
spec = ExperimentRolloutSerializer.to_spec(rollout, author)
update_flag(
experiment.environment,
experiment.feature,
FlagChangeSet(
author=spec.author,
enabled=True,
feature_state_value=spec.feature_state_value,
type_=spec.value_type,
segment_id=experiment.rollout_segment_id,
),
)


def mark_warehouse_pending_connection(
connection: WarehouseConnection,
) -> WarehouseConnection:
Expand Down
5 changes: 4 additions & 1 deletion api/experimentation/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from rest_framework.viewsets import GenericViewSet

from app.pagination import CustomPagination
from core.dataclasses import AuthorData
from environments.views import NestedEnvironmentViewSet
from experimentation.constants import (
EXPOSURES_REFRESH_MIN_INTERVAL,
Expand Down Expand Up @@ -302,7 +303,9 @@ def rollout(self, request: Request, **kwargs: object) -> Response:
serializer.is_valid(raise_exception=True)
apply_experiment_rollout(
experiment,
ExperimentRolloutSerializer.to_spec(serializer.validated_data, request),
ExperimentRolloutSerializer.to_spec(
serializer.validated_data, AuthorData.from_request(request)
),
)
return Response(self.get_serializer(experiment).data)

Expand Down
44 changes: 44 additions & 0 deletions api/tests/unit/experimentation/test_experiment_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@

from audit.models import AuditLog
from audit.related_object_type import RelatedObjectType
from core.dataclasses import AuthorData
from environments.models import Environment
from experimentation.constants import (
EXPERIMENT_FLAG,
EXPOSURES_REFRESH_MIN_INTERVAL,
RESULTS_REFRESH_MIN_INTERVAL,
)
from experimentation.dataclasses import RolloutSpec
from experimentation.models import (
ExpectedDirection,
Experiment,
Expand All @@ -31,17 +33,20 @@
Metric,
)
from experimentation.serializers import ExperimentFeatureSerializer
from experimentation.services import apply_experiment_rollout
from features.feature_types import MULTIVARIATE
from features.models import Feature, FeatureState
from features.multivariate.models import (
MultivariateFeatureOption,
MultivariateFeatureStateValue,
)
from features.versioning.dataclasses import MultivariateValueChangeSet
from segments.models import Condition
from tests.types import EnableFeaturesFixture

if TYPE_CHECKING:
from projects.models import Project
from users.models import FFAdminUser

pytestmark = pytest.mark.django_db

Expand Down Expand Up @@ -652,6 +657,45 @@ def test_action__start__sets_started_at(
assert response.json()["started_at"] is not None


def test_action_start__disabled_rollout__enables_override(
admin_client_new: APIClient,
environment: Environment,
experiment: Experiment,
multivariate_options: list[MultivariateFeatureOption],
admin_user: FFAdminUser,
enable_features: EnableFeaturesFixture,
) -> None:
# Given a configured but disabled rollout
enable_features(EXPERIMENT_FLAG)
option_a, option_b, _ = multivariate_options
apply_experiment_rollout(
experiment,
RolloutSpec(
enabled=False,
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),
),
)

# When the experiment is started
response = admin_client_new.post(_action_url(environment, experiment, "start"))

# Then the rollout segment override is enabled
assert response.status_code == status.HTTP_200_OK
override = FeatureState.objects.get(
environment=environment,
feature=experiment.feature,
feature_segment__segment=experiment.rollout_segment,
)
assert override.enabled is True


def test_action__complete__sets_ended_at(
admin_client_new: APIClient,
environment: Environment,
Expand Down
116 changes: 116 additions & 0 deletions api/tests/unit/experimentation/test_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
from features.multivariate.models import MultivariateFeatureOption
from features.value_types import STRING
from features.versioning.dataclasses import MultivariateValueChangeSet
from features.versioning.models import EnvironmentFeatureVersion
from segments.models import Condition
from users.models import FFAdminUser

Expand Down Expand Up @@ -1574,3 +1575,118 @@ def test_get_experiment_rollout__boolean_value__returns_lowercase_string(
# Then
assert rollout is not None
assert rollout["feature_state_value"] == {"type": "boolean", "value": "true"}


def test_enable_experiment_rollout__disabled_override__enables(
experiment: Experiment,
multivariate_options: list[MultivariateFeatureOption],
admin_user: FFAdminUser,
) -> None:
# Given a configured but disabled rollout
option_a, option_b, _ = multivariate_options
services.apply_experiment_rollout(
experiment,
RolloutSpec(
enabled=False,
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),
),
)

# When
services.enable_experiment_rollout(experiment, AuthorData(user=admin_user))

# Then the override is live with its value and allocations preserved
rollout = services.get_experiment_rollout(experiment)
assert rollout is not None
assert rollout["enabled"] is True
assert rollout["rollout_percentage"] == 20.0
assert rollout["feature_state_value"] == {"type": "string", "value": "control"}
assert {
(mv["multivariate_feature_option"], mv["percentage_allocation"])
for mv in rollout["multivariate_feature_state_values"]
} == {(option_a.id, 50.0), (option_b.id, 50.0)}


def test_enable_experiment_rollout__already_enabled__does_not_write(
experiment_with_rollout: Experiment,
admin_user: FFAdminUser,
mocker: MockerFixture,
) -> None:
# Given a rollout that is already enabled (from the fixture)
update_flag_spy = mocker.patch("experimentation.services.update_flag")

# When
services.enable_experiment_rollout(
experiment_with_rollout, AuthorData(user=admin_user)
)

# Then no flag write happens
update_flag_spy.assert_not_called()


def test_enable_experiment_rollout__no_rollout__does_not_write(
experiment: Experiment,
admin_user: FFAdminUser,
mocker: MockerFixture,
) -> None:
# Given an experiment without a rollout
update_flag_spy = mocker.patch("experimentation.services.update_flag")

# When
services.enable_experiment_rollout(experiment, AuthorData(user=admin_user))

# Then
update_flag_spy.assert_not_called()


def test_enable_experiment_rollout__v2_versioning__publishes_enabled_override(
environment_v2_versioning: Environment,
multivariate_feature: Feature,
multivariate_options: list[MultivariateFeatureOption],
admin_user: FFAdminUser,
) -> None:
# Given a disabled rollout on a v2 environment
option_a, _, _ = multivariate_options
experiment = Experiment.objects.create(
environment=environment_v2_versioning,
feature=multivariate_feature,
name="exp",
hypothesis="h",
status=ExperimentStatus.CREATED,
)
services.apply_experiment_rollout(
experiment,
RolloutSpec(
enabled=False,
rollout_percentage=30.0,
feature_state_value="control",
value_type="string",
multivariate_values=[MultivariateValueChangeSet(option_a.id, 60.0)],
author=AuthorData(user=admin_user),
),
)
versions_before = EnvironmentFeatureVersion.objects.filter(
environment=environment_v2_versioning, feature=multivariate_feature
).count()

# When
services.enable_experiment_rollout(experiment, AuthorData(user=admin_user))

# Then a new version is published that enables the override, value preserved
assert (
EnvironmentFeatureVersion.objects.filter(
environment=environment_v2_versioning, feature=multivariate_feature
).count()
== versions_before + 1
)
rollout = services.get_experiment_rollout(experiment)
assert rollout is not None
assert rollout["enabled"] is True
assert rollout["feature_state_value"] == {"type": "string", "value": "control"}
Loading