Merge pull request #80 from reddit/drop_variantless_expose_events

Return from `exposure()` if `variant_name` param not passed in

Author: mrlevitas

reddit_decider/__init__.py

@@ -388,6 +388,9 @@ def expose(
             to :code:`events_logger` (keys must be part of v2 event schema,
             use dicts for nested fields) under :code:`inputs` and as :code:`kwargs`
         """
+        if variant_name is None or variant_name == "":
+            return
+
         decider = self._get_decider()
         if decider is None:
             return
@@ -859,12 +862,12 @@ def get_all_dynamic_configs(self) -> List[Dict[str, Any]]:
         return parsed_configs
 
     def get_experiment(self, experiment_name: str) -> Optional[ExperimentConfig]:
-        """Get an :py:class:`~reddit_decider.ExperimentConfig` `dataclass <https://github.com/reddit/experiments.py/blob/728a9501faceab7072f9d62f4e391fa4c34a68b1/reddit_decider/__init__.py#L39-L47>`_
+        """Get an :py:class:`~reddit_decider.ExperimentConfig` `dataclass <https://github.com/reddit/experiments.py/blob/develop/reddit_decider/__init__.py#L44>`_
         representation of an experiment or :code:`None` if not found.
 
         :param experiment_name: Name of the experiment to be fetched.
 
-        :return: an :py:class:`~reddit_decider.ExperimentConfig` `dataclass <https://github.com/reddit/experiments.py/blob/728a9501faceab7072f9d62f4e391fa4c34a68b1/reddit_decider/__init__.py#L39-L47>`_
+        :return: an :py:class:`~reddit_decider.ExperimentConfig` `dataclass <https://github.com/reddit/experiments.py/blob/develop/reddit_decider/__init__.py#L44>`_
             representation of an experiment if found, else :code:`None`.
         """
         decider = self._get_decider()

reddit_experiments/__init__.py

@@ -301,6 +301,9 @@ def expose(
         :param kwargs: Additional arguments that will be passed to logger.
 
         """
+        if variant_name is None or variant_name == "":
+            return
+
         experiment = self._get_experiment(experiment_name)
 
         if experiment is None:

tests/decider_tests.py

@@ -749,6 +749,17 @@ def test_expose(self):
                 experiment_name="exp_1", variant=variant, event_fields=event_fields
             )
 
+    def test_expose_without_variant_name(self):
+        with create_temp_config_file(self.exp_base_config) as f:
+            decider = setup_decider(f.name, self.dc, self.mock_span, self.event_logger)
+
+            self.assertEqual(self.event_logger.log.call_count, 0)
+
+            decider.expose("exp_1", None)
+
+            # exposure assertions
+            self.assertEqual(self.event_logger.log.call_count, 0)
+
     def test_get_variant_for_identifier_without_expose_user_id(self):
         identifier = USER_ID
         bucket_val = "user_id"

tests/experiment_tests.py

@@ -259,6 +259,38 @@ def test_exposure_event_fields_with_cfg_data(self):
         self.assertEqual(getattr(event_fields["experiment"], "owner"), "test_owner")
         self.assertEqual(getattr(event_fields["experiment"], "version"), "1")
 
+    def test_expose_without_variant_name(self):
+        cfg_data = {
+            "test": {
+                "id": 1,
+                "name": "test",
+                "owner": "test_owner",
+                "type": "r2",
+                "version": "1",
+                "start_ts": time.time() - THIRTY_DAYS,
+                "stop_ts": time.time() + THIRTY_DAYS,
+                "experiment": {
+                    "id": 1,
+                    "name": "test",
+                    "variants": {"active": 10, "control_1": 10, "control_2": 10},
+                },
+            }
+        }
+        experiments = Experiments(
+            config_watcher=self.mock_filewatcher,
+            server_span=self.mock_span,
+            context_name="test",
+            cfg_data=cfg_data,
+            global_cache={},
+            event_logger=self.event_logger,
+        )
+
+        self.assertEqual(self.event_logger.log.call_count, 0)
+
+        experiments.expose("test", variant_name=None, user=self.user, app_name="r2")
+
+        self.assertEqual(self.event_logger.log.call_count, 0)
+
     def test_that_override_true_has_no_effect_with_cfg_data(self):
         cfg_data = {
             "test": {