Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
6571cfc
feat(nimbus): add weekly Celery task to update holdback enrollment pe…
Jun 24, 2026
0729b50
test(nimbus): add tests for update_holdback_enrollment_period task
Jun 24, 2026
e85d076
fix(nimbus): use computed_enrollment_end_date in holdback task and fi…
Jun 25, 2026
932752b
test(nimbus): add error path coverage for update_holdback_enrollment_…
Jun 29, 2026
d286993
style(nimbus): reformat test_raises_on_unexpected_error with parenthe…
Jun 29, 2026
3f31ba2
fix(nimbus): rework holdback task and serializer based on review feed…
Jun 30, 2026
c83d3b8
chore(docs): update OpenAPI schema and Swagger UI
Jun 30, 2026
5ae8bf2
fix(nimbus): use queryset update to avoid protected attribute access …
Jun 30, 2026
5e9abd2
fix(nimbus): use public properties and ISO format strings in holdback…
Jun 30, 2026
5936856
test(nimbus): add coverage for holdback serializer method overrides
Jun 30, 2026
f39cfaa
Merge branch 'main' into 16086
yashikakhurana Jul 7, 2026
01c4729
test(nimbus): add coverage for non-holdback proposedEnrollment fallba…
Jul 7, 2026
72fe48b
fix(nimbus): address review feedback on holdback task and serializer
Jul 7, 2026
abbe729
style(nimbus): format v8 serializer
Jul 7, 2026
7b38002
fix(nimbus): remove dead get_application method from v8 serializer
Jul 7, 2026
6700eea
fix(nimbus): run holdback task weekly with 28-day minimum (7 enrollme…
Jul 8, 2026
db296aa
fix(nimbus): run holdback task daily but trigger weekly per experimen…
Jul 8, 2026
d35ce7a
test(nimbus): add test for holdback do_rerun at 35-day boundary
Jul 8, 2026
38cafd8
fix(nimbus): set do_rerun once then update only timestamp weekly, ski…
Jul 8, 2026
27198fc
fix(nimbus): move weekly interval to HOLDBACK_RERUN_INTERVAL_DAYS con…
Jul 8, 2026
60abfd9
style(nimbus): format tasks.py
Jul 8, 2026
d1a933c
fix(nimbus): run holdback task at 3am UTC, simplify save, add debug l…
Jul 8, 2026
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: 2 additions & 4 deletions docs/experimenter/openapi-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1633,11 +1633,11 @@
},
"enrollmentEndDate": {
"type": "string",
"format": "date"
"readOnly": true
},
"endDate": {
"type": "string",
"format": "date"
"readOnly": true
},
"proposedDuration": {
"type": "string",
Expand Down Expand Up @@ -1697,8 +1697,6 @@
"channel",
"bucketConfig",
"startDate",
"enrollmentEndDate",
"endDate",
"publishedDate"
]
},
Expand Down
6 changes: 2 additions & 4 deletions docs/experimenter/swagger-ui.html
Original file line number Diff line number Diff line change
Expand Up @@ -1645,11 +1645,11 @@
},
"enrollmentEndDate": {
"type": "string",
"format": "date"
"readOnly": true
},
"endDate": {
"type": "string",
"format": "date"
"readOnly": true
},
"proposedDuration": {
"type": "string",
Expand Down Expand Up @@ -1709,8 +1709,6 @@
"channel",
"bucketConfig",
"startDate",
"enrollmentEndDate",
"endDate",
"publishedDate"
]
},
Expand Down
35 changes: 32 additions & 3 deletions experimenter/experimenter/experiments/api/v8/serializers.py
Comment thread
yashikakhurana marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import contextlib
import datetime
import json

from django.conf import settings
Expand Down Expand Up @@ -102,10 +103,10 @@ class NimbusExperimentSerializer(serializers.ModelSerializer):
outcomes = serializers.SerializerMethodField()
segments = serializers.SerializerMethodField()
startDate = serializers.DateField(source="start_date")
enrollmentEndDate = serializers.DateField(source="actual_enrollment_end_date")
endDate = serializers.DateField(source="end_date")
enrollmentEndDate = serializers.SerializerMethodField()
endDate = serializers.SerializerMethodField()
proposedDuration = serializers.ReadOnlyField(source="proposed_duration")
proposedEnrollment = serializers.ReadOnlyField(source="proposed_enrollment")
proposedEnrollment = serializers.SerializerMethodField()
referenceBranch = serializers.SerializerMethodField()
featureValidationOptOut = serializers.ReadOnlyField(
source="is_client_schema_disabled"
Expand Down Expand Up @@ -161,6 +162,34 @@ class Meta:
"requiresRestart",
)

def _holdback_enrollment_end(self, obj):
if obj.is_holdback and not obj.end_date and not obj.actual_enrollment_end_date:
return datetime.date.today() - datetime.timedelta(
days=settings.HOLDBACK_OBSERVATION_DAYS
)
return None

def get_enrollmentEndDate(self, obj):
holdback_end = self._holdback_enrollment_end(obj)
if holdback_end:
return holdback_end.isoformat()
enrollment_end = obj.actual_enrollment_end_date
return enrollment_end.isoformat() if enrollment_end else None

def get_endDate(self, obj):
holdback_end = self._holdback_enrollment_end(obj)
if holdback_end:
return (
holdback_end + datetime.timedelta(days=settings.HOLDBACK_OBSERVATION_DAYS)
).isoformat()
return obj.end_date.isoformat() if obj.end_date else None

def get_proposedEnrollment(self, obj):
holdback_end = self._holdback_enrollment_end(obj)
if holdback_end and obj.start_date:
return (holdback_end - obj.start_date).days
return obj.proposed_enrollment

def get_appName(self, obj):
return obj.application_config.app_name

Expand Down
1 change: 1 addition & 0 deletions experimenter/experimenter/experiments/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -3307,6 +3307,7 @@ class Messages:
COMPLETED = "Experiment is complete"
RESULTS_UPDATED = "Experiment results updated"
MONITORING_DATA_UPDATED = "Experiment monitoring data updated"
HOLDBACK_ENROLLMENT_UPDATED = "Holdback enrollment period updated"
EXPIRED_FROM_PREVIEW = "Expired from preview collection after 30 days"
REMOVED_FROM_PREVIEW = "Removed from preview collection"
PUSHED_TO_PREVIEW = "Pushed to preview collection"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,34 @@ def test_localized_localizations_json(self, l10n_json, expected):
else:
self.assertEqual(serializer.data["localizations"], expected)

def test_holdback_serializer_overrides(self):
today = datetime.date.today()
start = today - datetime.timedelta(days=50)
expected_enrollment_end = today - datetime.timedelta(days=21)
experiment = NimbusExperimentFactory.create_with_lifecycle(
NimbusExperimentFactory.Lifecycles.LIVE_ENROLLING,
is_holdback=True,
_start_date=start,
)
serializer = NimbusExperimentSerializer(experiment)
data = serializer.data

self.assertEqual(data["enrollmentEndDate"], expected_enrollment_end.isoformat())
self.assertEqual(data["endDate"], today.isoformat())
self.assertEqual(
data["proposedEnrollment"],
(expected_enrollment_end - start).days,
)

def test_non_holdback_proposed_enrollment_uses_model_value(self):
experiment = NimbusExperimentFactory.create_with_lifecycle(
NimbusExperimentFactory.Lifecycles.LIVE_ENROLLING,
is_holdback=False,
proposed_enrollment=14,
)
serializer = NimbusExperimentSerializer(experiment)
self.assertEqual(serializer.data["proposedEnrollment"], 14)

def _experiment_data_without_branches_and_featureIds(
self, experiment_data, min_required_version
) -> dict[str, Any]:
Expand Down
57 changes: 57 additions & 0 deletions experimenter/experimenter/jetstream/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,3 +196,60 @@ def fetch_monitoring_data():
metrics.incr("fetch_monitoring_data.failed")
logger.exception(f"Fatal error in fetch_monitoring_data task: {e}")
raise


@app.task
@metrics.timer_decorator("update_holdback_enrollment_period")
def update_holdback_enrollment_period():
metrics.incr("update_holdback_enrollment_period.started")
try:
today = timezone.now().date()
now = timezone.now()

experiments = NimbusExperiment.objects.filter(
is_holdback=True,
status=NimbusExperiment.Status.LIVE,
_end_date=None,
_enrollment_end_date=None,
).exclude(_start_date=None)
Comment thread
yashikakhurana marked this conversation as resolved.

minimum_days = (
settings.HOLDBACK_OBSERVATION_DAYS + settings.HOLDBACK_MINIMUM_ENROLLMENT_DAYS
)
updated_count = 0
for experiment in experiments:
days_since_start = (today - experiment.start_date).days
if (
days_since_start < minimum_days
or days_since_start % settings.HOLDBACK_RERUN_INTERVAL_DAYS != 0
):
logger.debug(
f"Skipping holdback {experiment.slug}: "
f"days_since_start={days_since_start}, "
f"minimum={minimum_days}, "
f"interval={settings.HOLDBACK_RERUN_INTERVAL_DAYS}"
)
continue
Comment thread
yashikakhurana marked this conversation as resolved.

save_fields = ["do_rerun_timestamp"]
if not experiment.do_rerun:
experiment.do_rerun = True
save_fields.append("do_rerun")
experiment.do_rerun_timestamp = now
experiment.save(update_fields=save_fields)
generate_nimbus_changelog(
experiment,
get_kinto_user(),
message=NimbusChangeLog.Messages.HOLDBACK_ENROLLMENT_UPDATED,
)
updated_count += 1

logger.info(
f"update_holdback_enrollment_period: updated {updated_count} experiments"
)
Comment thread
yashikakhurana marked this conversation as resolved.
metrics.incr("update_holdback_enrollment_period.completed")

except Exception as e:
metrics.incr("update_holdback_enrollment_period.failed")
logger.exception(f"Fatal error in update_holdback_enrollment_period: {e}")
raise
120 changes: 120 additions & 0 deletions experimenter/experimenter/jetstream/tests/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -3949,3 +3949,123 @@ def test_result_is_cached(self):
get_featmon_slugs()
get_featmon_slugs()
mock_read.assert_called_once()


class TestUpdateHoldbackEnrollmentPeriod(TestCase):
def test_sets_do_rerun_on_first_trigger(self):
today = datetime.date.today()
# 28 days ago: first trigger — sets both do_rerun and do_rerun_timestamp
experiment = NimbusExperimentFactory.create_with_lifecycle(
NimbusExperimentFactory.Lifecycles.LIVE_ENROLLING,
is_holdback=True,
_start_date=today - datetime.timedelta(days=28),
proposed_enrollment=14,
proposed_duration=84,
)
tasks.update_holdback_enrollment_period()
experiment.refresh_from_db()

self.assertTrue(experiment.do_rerun)
self.assertIsNotNone(experiment.do_rerun_timestamp)

def test_updates_only_timestamp_on_subsequent_triggers(self):
today = datetime.date.today()
# 35 days: do_rerun already True — only do_rerun_timestamp should update
experiment = NimbusExperimentFactory.create_with_lifecycle(
NimbusExperimentFactory.Lifecycles.LIVE_ENROLLING,
is_holdback=True,
_start_date=today - datetime.timedelta(days=35),
proposed_enrollment=14,
proposed_duration=84,
do_rerun=True,
)
tasks.update_holdback_enrollment_period()
experiment.refresh_from_db()

self.assertTrue(experiment.do_rerun)
self.assertIsNotNone(experiment.do_rerun_timestamp)

def test_skips_holdback_with_enrollment_stopped(self):
today = datetime.date.today()
experiment = NimbusExperimentFactory.create_with_lifecycle(
NimbusExperimentFactory.Lifecycles.LIVE_ENROLLING,
is_holdback=True,
_start_date=today - datetime.timedelta(days=35),
_enrollment_end_date=today - datetime.timedelta(days=7),
proposed_enrollment=14,
proposed_duration=84,
)
tasks.update_holdback_enrollment_period()
experiment.refresh_from_db()

self.assertFalse(experiment.do_rerun)

def test_skips_experiment_below_minimum_days(self):
today = datetime.date.today()
# Started 20 days ago: 20 < 28 (minimum), so should be skipped
experiment = NimbusExperimentFactory.create_with_lifecycle(
NimbusExperimentFactory.Lifecycles.LIVE_ENROLLING,
is_holdback=True,
_start_date=today - datetime.timedelta(days=20),
proposed_enrollment=14,
proposed_duration=84,
)
tasks.update_holdback_enrollment_period()
experiment.refresh_from_db()

self.assertFalse(experiment.do_rerun)

def test_skips_experiment_not_on_weekly_boundary(self):
today = datetime.date.today()
# Started 29 days ago: >= 28 but 29 % 7 != 0, so should be skipped
experiment = NimbusExperimentFactory.create_with_lifecycle(
NimbusExperimentFactory.Lifecycles.LIVE_ENROLLING,
is_holdback=True,
_start_date=today - datetime.timedelta(days=29),
proposed_enrollment=14,
proposed_duration=84,
)
tasks.update_holdback_enrollment_period()
experiment.refresh_from_db()

self.assertFalse(experiment.do_rerun)

def test_skips_ended_holdback(self):
today = datetime.date.today()
experiment = NimbusExperimentFactory.create_with_lifecycle(
NimbusExperimentFactory.Lifecycles.LIVE_ENROLLING,
is_holdback=True,
_start_date=today - datetime.timedelta(days=50),
proposed_enrollment=14,
proposed_duration=84,
)
experiment._end_date = today - datetime.timedelta(days=1)
experiment.save(update_fields=["_end_date"])
tasks.update_holdback_enrollment_period()
experiment.refresh_from_db()

self.assertFalse(experiment.do_rerun)

def test_skips_non_holdback_experiments(self):
today = datetime.date.today()
experiment = NimbusExperimentFactory.create_with_lifecycle(
NimbusExperimentFactory.Lifecycles.LIVE_ENROLLING,
is_holdback=False,
_start_date=today - datetime.timedelta(days=50),
proposed_enrollment=14,
proposed_duration=84,
)
tasks.update_holdback_enrollment_period()
experiment.refresh_from_db()

self.assertFalse(experiment.do_rerun)

def test_raises_on_unexpected_error(self):
with (
patch(
"experimenter.jetstream.tasks.NimbusExperiment.objects.filter",
side_effect=Exception("db error"),
),
self.assertRaises(Exception, msg="db error"),
):
tasks.update_holdback_enrollment_period()
8 changes: 8 additions & 0 deletions experimenter/experimenter/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,10 @@
API_CACHE_WARMING_TTL = 60 * 60 * 24
SIZING_DATA_KEY = "population_sizing"

HOLDBACK_OBSERVATION_DAYS = 21
HOLDBACK_MINIMUM_ENROLLMENT_DAYS = 7
HOLDBACK_RERUN_INTERVAL_DAYS = 7

# Celery
CELERY_BROKER_URL = f"redis://{REDIS_HOST}:{REDIS_PORT}/{REDIS_DB}"
CELERY_BEAT_SCHEDULE = {
Expand Down Expand Up @@ -421,6 +425,10 @@
"task": "experimenter.experiments.tasks.warm_api_caches",
"schedule": config("API_CACHE_WARMING_INTERVAL", default=3600, cast=int),
},
"update_holdback_enrollment_period": {
"task": "experimenter.jetstream.tasks.update_holdback_enrollment_period",
"schedule": crontab(minute=0, hour=3),
},
Comment thread
yashikakhurana marked this conversation as resolved.
}
CELERY_TASK_ROUTES = {
"experimenter.kinto.tasks.*": {"queue": "remote_settings"},
Expand Down
Loading