From 6c869139fa08e7a21718a689552c8f4462fc68f3 Mon Sep 17 00:00:00 2001 From: Tif Tran Date: Mon, 27 Jul 2020 13:40:35 -0700 Subject: [PATCH] hardcoded count for bucket config fixes #3014 (#3091) * generate bucketConfig when accepted to rs fixes #3014 * feedback changes * changes to use nimbus schema validation, changes randomizationUnit to preset value * docs * feedback changes * formatting and deleting experimentRecipe.json * unused import --- .../experiments/api/v4/serializers.py | 22 +++ .../tests/api/v4/experimentRecipe.json | 163 ------------------ .../tests/api/v4/test_serializers.py | 86 +++++++-- app/experimenter/kinto/tasks.py | 15 +- app/experimenter/kinto/tests/test_tasks.py | 21 ++- 5 files changed, 132 insertions(+), 175 deletions(-) delete mode 100644 app/experimenter/experiments/tests/api/v4/experimentRecipe.json diff --git a/app/experimenter/experiments/api/v4/serializers.py b/app/experimenter/experiments/api/v4/serializers.py index 6d7c91352c..f23b731b8c 100644 --- a/app/experimenter/experiments/api/v4/serializers.py +++ b/app/experimenter/experiments/api/v4/serializers.py @@ -5,6 +5,7 @@ from experimenter.experiments.models import ( Experiment, ExperimentVariant, + ExperimentBucketRange, ) NIMBUS_DATA = get_data() @@ -22,6 +23,24 @@ def get_value(self, obj): return None +class ExperimentBucketRangeSerializer(serializers.ModelSerializer): + namespace = serializers.SlugRelatedField(read_only=True, slug_field="name") + randomizationUnit = serializers.SerializerMethodField() + total = serializers.SerializerMethodField() + + class Meta: + model = ExperimentBucketRange + fields = ("randomizationUnit", "namespace", "start", "count", "total") + + def get_randomizationUnit(self, obj): + return NIMBUS_DATA["ExperimentDesignPresets"]["empty_aa"]["preset"]["arguments"][ + "bucketConfig" + ]["randomizationUnit"] + + def get_total(self, obj): + return obj.namespace.total + + class ExperimentRapidArgumentSerializer(serializers.ModelSerializer): slug = serializers.ReadOnlyField(source="normandy_slug") userFacingName = serializers.ReadOnlyField(source="name") @@ -53,6 +72,9 @@ class Meta: ) def get_bucketConfig(self, obj): + + if hasattr(obj, "bucket"): + return ExperimentBucketRangeSerializer(obj.bucket).data return { "randomizationUnit": "normandy_id", "namespace": "", diff --git a/app/experimenter/experiments/tests/api/v4/experimentRecipe.json b/app/experimenter/experiments/tests/api/v4/experimentRecipe.json deleted file mode 100644 index adb52ef3b3..0000000000 --- a/app/experimenter/experiments/tests/api/v4/experimentRecipe.json +++ /dev/null @@ -1,163 +0,0 @@ -{ - "$schema": "http://json-schema.org/draft-07/schema#", - "$ref": "#/definitions/ExperimentRecipe", - "definitions": { - "ExperimentRecipe": { - "type": "object", - "properties": { - "id": { - "type": "string", - "description": "A unique identifier for the Recipe" - }, - "filter_expression": { - "type": "string", - "description": "JEXL expression defined in an Audience" - }, - "targeting": { - "type": "string", - "description": "JEXL expression using messaging system environment" - }, - "enabled": { - "type": "boolean", - "description": "Is the experiment enabled?" - }, - "arguments": { - "$ref": "#/definitions/Experiment", - "description": "Experiment definition" - } - }, - "required": ["id", "filter_expression", "enabled", "arguments"], - "additionalProperties": false, - "description": "The experiment definition accessible to Firefox via Remote Settings.\nIt is compatible with ExperimentManager." - }, - "Experiment": { - "type": "object", - "properties": { - "slug": { - "type": "string", - "description": "Unique identifier for the experiment" - }, - "userFacingName": { - "type": "string", - "description": "Publically-accesible name of the experiment" - }, - "userFacingDescription": { - "type": "string", - "description": "Short public description of the experiment" - }, - "active": { - "type": "boolean", - "description": "Is the experiment currently live in production? i.e., published to remote settings?" - }, - "isEnrollmentPaused": { - "type": "boolean", - "description": "Are we continuing to enroll new users into the experiment?" - }, - "bucketConfig": { - "type": "object", - "properties": { - "randomizationUnit": { - "type": "string", - "enum": ["client_id", "normandy_id"], - "description": "The randomization unit. Note that client_id is not yet implemented.", - "default": "normandy_id" - }, - "namespace": { - "type": "string", - "description": "Additional inputs to the hashing function" - }, - "start": { - "type": "number", - "description": "Index of start of the range of buckets" - }, - "count": { - "type": "number", - "description": "Number of buckets to check" - }, - "total": { - "type": "number", - "description": "Total number of buckets", - "default": 10000 - } - }, - "required": [ - "randomizationUnit", - "namespace", - "start", - "count", - "total" - ], - "additionalProperties": false, - "description": "Bucketing configuration" - }, - "features": { - "type": "array", - "items": { "type": "string" }, - "description": "A list of features relevant to the experiment analysis" - }, - "branches": { - "type": "array", - "items": { - "type": "object", - "properties": { - "slug": { - "type": "string", - "description": "Identifier for the branch" - }, - "ratio": { - "type": "number", - "description": "Relative ratio of population for the branch (e.g. if branch A=1 and branch B=3,\nbranch A would get 25% of the population)", - "default": 1 - }, - "group": { - "type": "array", - "items": { "type": "string", "enum": ["cfr", "aboutwelcome"] }, - "description": "Used to indicate a type of branch value" - }, - "value": { - "anyOf": [{ "type": "object" }, { "type": "null" }], - "description": "The variant payload. TODO: This will be more strictly validated." - } - }, - "required": ["slug", "ratio", "value"], - "additionalProperties": false - }, - "description": "Branch configuration for the experiment" - }, - "startDate": { - "type": "string", - "description": "Actual publish date of the experiment", - "format": "date-time" - }, - "endDate": { - "type": ["string", "null"], - "description": "Actual end date of the experiment", - "format": "date-time" - }, - "proposedEnrollment": { - "type": "number", - "description": "Duration of enrollment from the start date in days" - }, - "referenceBranch": { - "type": ["string", "null"], - "description": "The slug of the reference branch" - } - }, - "required": [ - "slug", - "userFacingName", - "userFacingDescription", - "active", - "isEnrollmentPaused", - "bucketConfig", - "features", - "branches", - "startDate", - "endDate", - "proposedEnrollment", - "referenceBranch" - ], - "additionalProperties": false - } - } -} diff --git a/app/experimenter/experiments/tests/api/v4/test_serializers.py b/app/experimenter/experiments/tests/api/v4/test_serializers.py index 4cc74cca41..a5a133b289 100644 --- a/app/experimenter/experiments/tests/api/v4/test_serializers.py +++ b/app/experimenter/experiments/tests/api/v4/test_serializers.py @@ -1,11 +1,8 @@ import datetime -import json -import os -from jsonschema import validate from django.test import TestCase -from experimenter.experiments.models import Experiment +from experimenter.experiments.models import Experiment, ExperimentBucketNamespace from experimenter.experiments.tests.factories import ( ExperimentFactory, ExperimentVariantFactory, @@ -38,12 +35,6 @@ def test_serializer_outputs_expected_schema(self): serializer = ExperimentRapidRecipeSerializer(experiment) data = serializer.data - fn = os.path.join(os.path.dirname(__file__), "experimentRecipe.json") - - with open(fn, "r") as f: - json_schema = json.load(f) - self.assertIsNone(validate(instance=data, schema=json_schema)) - arguments = data.pop("arguments") branches = arguments.pop("branches") @@ -88,3 +79,78 @@ def test_serializer_outputs_expected_schema(self): {"ratio": 33, "slug": "control", "value": None}, ], ) + + def test_serializer_outputs_expected_schema_with_nameSpace_bucket(self): + audience = "us_only" + features = ["pinned_tabs", "picture_in_picture"] + normandy_slug = "experimenter-normandy-slug" + today = datetime.datetime.today() + experiment = ExperimentFactory.create( + type=Experiment.TYPE_RAPID, + rapid_type=Experiment.RAPID_AA_CFR, + audience=audience, + features=features, + normandy_slug=normandy_slug, + firefox_min_version="80.0", + proposed_enrollment=9, + proposed_start_date=today, + ) + + ExperimentVariantFactory.create( + experiment=experiment, slug="control", is_control=True + ) + ExperimentVariantFactory.create(experiment=experiment, slug="variant-2") + + ExperimentBucketNamespace.request_namespace_buckets( + experiment.normandy_slug, experiment, 100 + ) + + serializer = ExperimentRapidRecipeSerializer(experiment) + data = serializer.data + + arguments = data.pop("arguments") + branches = arguments.pop("branches") + + self.assertDictEqual( + data, + { + "id": normandy_slug, + "filter_expression": "env.version|versionCompare('80.0') >= 0", + "targeting": "localeLanguageCode == 'en' && region == 'US'" + " && browserSettings.update.channel == 'release'", + "enabled": True, + }, + ) + + bucket = experiment.bucket + + self.assertDictEqual( + dict(arguments), + { + "userFacingName": experiment.name, + "userFacingDescription": experiment.public_description, + "slug": normandy_slug, + "active": True, + "isEnrollmentPaused": False, + "endDate": None, + "proposedEnrollment": experiment.proposed_enrollment, + "features": features, + "referenceBranch": "control", + "startDate": today.isoformat(), + "bucketConfig": { + "count": bucket.count, + "namespace": bucket.namespace.name, + "randomizationUnit": "userId", + "start": bucket.start, + "total": bucket.namespace.total, + }, + }, + ) + converted_branches = [dict(branch) for branch in branches] + self.assertEqual( + converted_branches, + [ + {"ratio": 33, "slug": "variant-2", "value": None}, + {"ratio": 33, "slug": "control", "value": None}, + ], + ) diff --git a/app/experimenter/kinto/tasks.py b/app/experimenter/kinto/tasks.py index 490e2b7b7d..a4a7c2e6b5 100644 --- a/app/experimenter/kinto/tasks.py +++ b/app/experimenter/kinto/tasks.py @@ -4,16 +4,20 @@ from celery.utils.log import get_task_logger from django.conf import settings +from mozilla_nimbus_shared import get_data + from experimenter.celery import app from experimenter.experiments.api.v4.serializers import ExperimentRapidRecipeSerializer from experimenter.experiments.changelog_utils import update_experiment_with_change_log -from experimenter.experiments.models import Experiment +from experimenter.experiments.models import Experiment, ExperimentBucketNamespace from experimenter.kinto import client logger = get_task_logger(__name__) metrics = markus.get_metrics("kinto.tasks") +NIMBUS_DATA = get_data() + @app.task @metrics.timer_decorator("push_experiment_to_kinto.timing") @@ -21,7 +25,16 @@ def push_experiment_to_kinto(experiment_id): metrics.incr("push_experiment_to_kinto.started") experiment = Experiment.objects.get(id=experiment_id) + ExperimentBucketNamespace.request_namespace_buckets( + experiment.normandy_slug, + experiment, + NIMBUS_DATA["ExperimentDesignPresets"]["empty_aa"]["preset"]["arguments"][ + "bucketConfig" + ]["count"], + ) + data = ExperimentRapidRecipeSerializer(experiment).data + logger.info(f"Pushing {experiment} to Kinto") try: diff --git a/app/experimenter/kinto/tests/test_tasks.py b/app/experimenter/kinto/tests/test_tasks.py index 9a901e1f4f..67b4db5721 100644 --- a/app/experimenter/kinto/tests/test_tasks.py +++ b/app/experimenter/kinto/tests/test_tasks.py @@ -4,12 +4,16 @@ from django.conf import settings from django.test import TestCase -from experimenter.experiments.models import Experiment +from mozilla_nimbus_shared import get_data + +from experimenter.experiments.models import Experiment, ExperimentBucketRange from experimenter.experiments.tests.factories import ExperimentFactory from experimenter.kinto.tests.mixins import MockKintoClientMixin from experimenter.kinto import tasks from experimenter.experiments.api.v4.serializers import ExperimentRapidRecipeSerializer +NIMBUS_DATA = get_data() + class TestPushExperimentToKintoTask(MockKintoClientMixin, TestCase): def setUp(self): @@ -17,6 +21,7 @@ def setUp(self): self.experiment = ExperimentFactory.create_with_status( Experiment.STATUS_DRAFT, proposed_start_date=datetime.date(2020, 1, 20), + normandy_slug="normandy-slug", audience="us_only", ) @@ -25,6 +30,20 @@ def test_push_experiment_to_kinto_sends_experiment_data(self): data = ExperimentRapidRecipeSerializer(self.experiment).data + self.assertTrue( + ExperimentBucketRange.objects.filter(experiment=self.experiment).exists() + ) + + bucketConfig = data["arguments"]["bucketConfig"].copy() + bucketConfig.pop("start") + bucketConfig.pop("namespace") + + designPreset = NIMBUS_DATA["ExperimentDesignPresets"]["empty_aa"]["preset"][ + "arguments" + ]["bucketConfig"] + + self.assertEqual(bucketConfig, designPreset) + self.mock_kinto_client.create_record.assert_called_with( data=data, collection=settings.KINTO_COLLECTION,