RustDecider#choose() in get_variant_for_identifier*()

Author: mrlevitas

reddit_decider/__init__.py

@@ -483,38 +483,31 @@ def get_variant_for_identifier(
             )
             return None
 
-        decider = self._get_decider()
-        if decider is None:
-            return None
-
-        ctx = self._get_ctx_with_set_identifier(
-            identifier=identifier, identifier_type=identifier_type
-        )
-        ctx_err = ctx.err()
-        if ctx_err is not None:
-            logger.info(f"Encountered error in rust_decider.make_ctx(): {ctx_err}")
+        if self._internal is None:
+            logger.error("RustDecider is None--did not initialize.")
             return None
 
-        choice = decider.choose(
-            feature_name=experiment_name, ctx=ctx, identifier_type=identifier_type
-        )
-        error = choice.err()
+        ctx = self._decider_context.to_dict()
+        ctx[identifier_type] = identifier
 
-        if error:
-            logger.info(f"Encountered error in decider.choose(): {error}")
+        try:
+            decision = self._internal.choose(experiment_name, ctx)
+        except FeatureNotFoundException as exc:
+            warnings.warn(str(exc))
+            return None
+        except DeciderException as exc:
+            logger.info(str(exc))
             return None
-
-        variant = choice.decision()
 
         event_context_fields = self._decider_context.to_event_dict()
         event_context_fields.update(exposure_kwargs or {})
 
-        for event in choice.events():
+        for event in decision.events:
             self._send_expose(
                 event=event, exposure_fields=event_context_fields, overwrite_identifier=True
             )
 
-        return variant
+        return decision.variant
 
     def get_variant_for_identifier_without_expose(
         self,
@@ -549,37 +542,31 @@ def get_variant_for_identifier_without_expose(
             )
             return None
 
-        decider = self._get_decider()
-        if decider is None:
+        if self._internal is None:
+            logger.error("RustDecider is None--did not initialize.")
             return None
 
-        ctx = self._get_ctx_with_set_identifier(
-            identifier=identifier, identifier_type=identifier_type
-        )
-        ctx_err = ctx.err()
-        if ctx_err is not None:
-            logger.info(f"Encountered error in rust_decider.make_ctx(): {ctx_err}")
-            return None
+        ctx = self._decider_context.to_dict()
+        ctx[identifier_type] = identifier
 
-        choice = decider.choose(
-            feature_name=experiment_name, ctx=ctx, identifier_type=identifier_type
-        )
-        error = choice.err()
-        if error:
-            logger.info(f"Encountered error in decider.choose(): {error}")
+        try:
+            decision = self._internal.choose(experiment_name, ctx)
+        except FeatureNotFoundException as exc:
+            warnings.warn(str(exc))
+            return None
+        except DeciderException as exc:
+            logger.info(str(exc))
             return None
-
-        variant = choice.decision()
 
         event_context_fields = self._decider_context.to_event_dict()
 
         # expose Holdout if the experiment is part of one
-        for event in choice.events():
+        for event in decision.events:
             self._send_expose_if_holdout(
                 event=event, exposure_fields=event_context_fields, overwrite_identifier=True
             )
 
-        return variant
+        return decision.variant
 
     def get_all_variants_without_expose(self) -> List[Dict[str, Union[str, int]]]:
         """Return a list of experiment dicts in this format:

tests/decider_tests.py

@@ -708,31 +708,6 @@ def test_get_variant_for_identifier_device_id(self):
             # `identifier` passed to correct event field of experiment's `bucket_val` config
             self.assertEqual(event_fields["device_id"], identifier)
 
-    def test_get_variant_for_identifier_wrong_bucket_val(self):
-        identifier = USER_ID
-        bucket_val = "device_id"
-        self.exp_base_config["exp_1"]["experiment"].update({"bucket_val": bucket_val})
-
-        with create_temp_config_file(self.exp_base_config) as f:
-            decider = setup_decider(f, self.dc, self.mock_span, self.event_logger)
-
-            self.assertEqual(self.event_logger.log.call_count, 0)
-            with self.assertLogs() as captured:
-                # `identifier_type="canonical_url"`, which doesn't match `bucket_val` of `device_id`
-                variant = decider.get_variant_for_identifier(
-                    experiment_name="exp_1", identifier=identifier, identifier_type="canonical_url"
-                )
-                # `None` is returned since `identifier_type` doesn't match `bucket_val` in experiment-config json
-                self.assertEqual(variant, None)
-                # exposure isn't emitted either
-                self.assertEqual(self.event_logger.log.call_count, 0)
-
-                assert any(
-                    'Encountered error in decider.choose(): Requested identifier_type "canonical_url" is incompatible with experiment\'s bucket_val = device_id'
-                    in x.getMessage()
-                    for x in captured.records
-                )
-
     def test_get_variant_for_identifier_bogus_identifier_type(self):
         identifier = "anything"
         identifier_type = "blah"
@@ -816,36 +791,6 @@ def test_get_variant_for_identifier_without_expose_user_id_for_holdout_exposure(
                 experiment_name="hg", variant="holdout", event_fields=event_fields
             )
 
-    def test_get_variant_for_identifier_without_expose_for_holdout_exposure_wrong_bucket_val(self):
-        identifier = DEVICE_ID
-        bucket_val = "device_id"
-        self.exp_base_config["exp_1"]["experiment"].update({"bucket_val": bucket_val})
-
-        self.exp_base_config["exp_1"].update({"parent_hg_name": "hg"})
-        self.parent_hg_config["hg"]["experiment"].update({"bucket_val": bucket_val})
-        self.exp_base_config.update(self.parent_hg_config)
-
-        with create_temp_config_file(self.exp_base_config) as f:
-            decider = setup_decider(f, self.dc, self.mock_span, self.event_logger)
-
-            self.assertEqual(self.event_logger.log.call_count, 0)
-            # `identifier_type="canonical_url"`, which doesn't match `bucket_val` of `device_id`
-            self.assertEqual(self.event_logger.log.call_count, 0)
-            with self.assertLogs() as captured:
-                variant = decider.get_variant_for_identifier_without_expose(
-                    experiment_name="exp_1", identifier=identifier, identifier_type="canonical_url"
-                )
-                # `None` is returned since `identifier_type` doesn't match `bucket_val` in experiment-config json
-                self.assertEqual(variant, None)
-
-                assert any(
-                    'Encountered error in decider.choose(): Requested identifier_type "canonical_url" is incompatible with experiment\'s bucket_val = device_id'
-                    in x.getMessage()
-                    for x in captured.records
-                )
-
-            self.assertEqual(self.event_logger.log.call_count, 0)
-
     def test_get_variant_for_identifier_without_expose_canonical_url(self):
         identifier = CANONICAL_URL
         bucket_val = "canonical_url"