diff --git a/docs/requirements.txt b/docs/requirements.txt index 1289c9b..c4905ac 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -4,7 +4,7 @@ docutils==0.16 Jinja2==2.11.2 MarkupSafe==1.1.1 pydocstyle==5.1.1 -reddit-decider==1.7.0 +reddit-decider==1.11.0 reddit-edgecontext==1.0.0a3 Sphinx==3.4.0 sphinx-autodoc-typehints==1.11.1 diff --git a/reddit_decider/__init__.py b/reddit_decider/__init__.py index a7a242a..e65e532 100644 --- a/reddit_decider/__init__.py +++ b/reddit_decider/__init__.py @@ -73,8 +73,9 @@ class ExperimentConfig: bucket_val: str start_ts: int stop_ts: int - owner: str + owner: None = None emit_event: Optional[bool] = None + measured: Optional[bool] = None class DeciderContext: @@ -236,7 +237,6 @@ def _send_expose(self, event: str, exposure_fields: dict) -> None: bucket_val, start_ts, stop_ts, - owner, ) = event.split("::::") except ValueError: logger.warning( @@ -251,7 +251,6 @@ def _send_expose(self, event: str, exposure_fields: dict) -> None: bucket_val=bucket_val, start_ts=self._cast_to_int(start_ts), stop_ts=self._cast_to_int(stop_ts), - owner=owner, ) event_fields = {**event_fields, **{bucket_val: bucketing_value}} @@ -279,7 +278,6 @@ def _send_expose_if_holdout(self, event: str, exposure_fields: dict) -> None: bucket_val, start_ts, stop_ts, - owner, ) = event.split("::::") except ValueError: logger.warning( @@ -299,7 +297,6 @@ def _send_expose_if_holdout(self, event: str, exposure_fields: dict) -> None: bucket_val=bucket_val, start_ts=self._cast_to_int(start_ts), stop_ts=self._cast_to_int(stop_ts), - owner=owner, ) event_fields = {**event_fields, **{bucket_val: bucketing_value}} @@ -432,7 +429,6 @@ def expose( bucket_val=feature.bucket_val, start_ts=feature.start_ts, stop_ts=feature.stop_ts, - owner=feature.owner, ) self._event_logger.log( @@ -920,8 +916,8 @@ def get_experiment(self, experiment_name: str) -> Optional[ExperimentConfig]: bucket_val=feature.bucket_val, start_ts=feature.start_ts, stop_ts=feature.stop_ts, - owner=feature.owner, emit_event=feature.emit_event, + measured=feature.measured, ) diff --git a/requirements.txt b/requirements.txt index 5c41667..f795d7a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,7 +1,7 @@ -r requirements-transitive.txt baseplate>=2.0.0a1,<3.0 black==21.4b2 -reddit-decider==1.7.0 +reddit-decider==1.11.0 flake8==3.9.1 mypy==0.790 prometheus-client>=0.12.0,<1.0 diff --git a/setup.py b/setup.py index 43e8f32..364435b 100644 --- a/setup.py +++ b/setup.py @@ -19,7 +19,7 @@ install_requires=[ "baseplate>=2.0.0a1,<3.0", "reddit-edgecontext>=1.0.0a3,<2.0", - "reddit-decider~=1.7.0", + "reddit-decider~=1.11.0", "typing_extensions>=3.10.0.0,<5.0", ], package_data={"reddit_experiments": ["py.typed"], "reddit_decider": ["py.typed"]}, diff --git a/tests/decider_tests.py b/tests/decider_tests.py index e7fb2a7..65160c7 100644 --- a/tests/decider_tests.py +++ b/tests/decider_tests.py @@ -500,7 +500,7 @@ def assert_exposure_event_fields( cfg = self.exp_base_config[experiment_name] self.assertEqual(getattr(event_fields["experiment"], "id"), cfg["id"]) self.assertEqual(getattr(event_fields["experiment"], "name"), cfg["name"]) - self.assertEqual(getattr(event_fields["experiment"], "owner"), cfg["owner"]) + self.assertEqual(getattr(event_fields["experiment"], "owner"), None) self.assertEqual(getattr(event_fields["experiment"], "version"), cfg["version"]) self.assertEqual(getattr(event_fields["experiment"], "bucket_val"), bucket_val) @@ -520,7 +520,7 @@ def assert_minimal_exposure_event_fields( cfg = self.exp_base_config[experiment_name] self.assertEqual(getattr(event_fields["experiment"], "id"), cfg["id"]) self.assertEqual(getattr(event_fields["experiment"], "name"), cfg["name"]) - self.assertEqual(getattr(event_fields["experiment"], "owner"), cfg["owner"]) + self.assertEqual(getattr(event_fields["experiment"], "owner"), None) self.assertEqual(getattr(event_fields["experiment"], "version"), cfg["version"]) self.assertEqual(getattr(event_fields["experiment"], "bucket_val"), bucket_val) @@ -573,7 +573,7 @@ def test_none_returned_on_get_variant_call_with_bad_id(self): self.assertEqual(self.event_logger.log.call_count, 0) assert any( - "Partially loaded Decider: 1 features failed to load: {'test': 'Manifest parsing error: invalid type: string \"1\", expected u32'}" + 'Partially loaded Decider: 1 feature(s) failed to load: {"test": ParsingError(Error("invalid type: string \\"1\\", expected u32", line: 0, column: 0))' in x.getMessage() for x in captured.records ) @@ -1508,6 +1508,81 @@ def test_get_variant_with_disabled_exp(self): # exposure assertions self.assertEqual(self.event_logger.log.call_count, 0) + def test_range_variant_emit_event_override(self): + cfg = { + "feature_rollout_100": { + "id": 9110, + "name": "feature_rollout_100", + "enabled": True, + "version": "1", + "start_ts": 1522306800, + "stop_ts": 32533405261, + "owner": "test_user@reddit.com", + "type": "feature_rollout", + "emit_event": False, + "experiment": { + "variants": [ + {"name": "enabled", "size": 1.0, "range_end": 1.0, "range_start": 0.0} + ], + "experiment_version": 1, + "shuffle_version": 0, + "bucket_val": "user_id", + "log_bucketing": False, + }, + }, + "measured_rollout_100": { + "id": 9119, + "name": "measured_rollout_100", + "enabled": True, + "version": "1", + "start_ts": 1522306800, + "stop_ts": 32533405261, + "owner": "test_user@reddit.com", + "type": "range_variant", + "emit_event": False, + "measured": True, + "experiment": { + "variants": [ + { + "name": "enabled", + "size": 1.0, + "range_end": 1.0, + "range_start": 0.0, + "emit_event_override": True, + } + ], + "experiment_version": 1, + "shuffle_version": 0, + "bucket_val": "user_id", + "log_bucketing": False, + }, + }, + } + + with create_temp_config_file(cfg) as f: + decider = setup_decider(f, self.dc, self.mock_span, self.event_logger) + + self.assertEqual(self.event_logger.log.call_count, 0) + variant = decider.get_variant(experiment_name="feature_rollout_100") + self.assertEqual(variant, "enabled") + + # FR does NOT emit exposure + self.assertEqual(self.event_logger.log.call_count, 0) + + # FR is NOT "measured" + fr_cfg = decider.get_experiment(experiment_name="feature_rollout_100") + self.assertEqual(fr_cfg.measured, False) + + variant = decider.get_variant(experiment_name="measured_rollout_100") + self.assertEqual(variant, "enabled") + + # Measured Rollout DOES emit exposure (due to RV "emit_event_override" field) + self.assertEqual(self.event_logger.log.call_count, 1) + + # MR IS "measured" + mr_cfg = decider.get_experiment(experiment_name="measured_rollout_100") + self.assertEqual(mr_cfg.measured, True) + def test_get_experiment(self): with create_temp_config_file(self.exp_base_config) as f: decider = setup_decider(f, self.dc, self.mock_span, self.event_logger) @@ -1521,7 +1596,6 @@ def test_get_experiment(self): self.assertEqual(experiment.bucket_val, cfg["experiment"]["bucket_val"]) self.assertEqual(experiment.start_ts, cfg["start_ts"]) self.assertEqual(experiment.stop_ts, cfg["stop_ts"]) - self.assertEqual(experiment.owner, cfg["owner"]) self.assertEqual(experiment.emit_event, True) def test_get_variant_without_expose_with_HG_as_control_1_and_child_returns_none_does_expose(