Merge pull request #88 from reddit/incorporate_dc_from_shim

Add DCs from decider-py shim (metrics)

Author: mrlevitas

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.2.29
+reddit-decider==1.2.30
 reddit-edgecontext==1.0.0a3
 Sphinx==3.4.0
 sphinx-autodoc-typehints==1.11.1

reddit_decider/__init__.py

@@ -10,6 +10,8 @@
 from typing import IO
 from typing import List
 from typing import Optional
+from typing import Type
+from typing import TypeVar
 from typing import Union
 
 from baseplate import RequestContext
@@ -27,13 +29,15 @@
 from rust_decider import Decision
 from rust_decider import FeatureNotFoundException
 from rust_decider import make_ctx
+from rust_decider import ValueTypeMismatchException
 from typing_extensions import Literal
 
 
 logger = logging.getLogger(__name__)
 
 EMPLOYEE_ROLES = ["employee", "contractor"]
 IDENTIFIERS = ["user_id", "device_id", "canonical_url"]
+TYPE_STR_LOOKUP = {bool: "boolean", int: "integer", float: "float", str: "string", dict: "map"}
 
 
 class EventType(Enum):
@@ -58,6 +62,8 @@ class DeciderContext:
     :code:`DeciderContext()` is populated in :code:`make_object_for_context()`.
     """
 
+    T = TypeVar("T")
+
     def __init__(
         self,
         user_id: Optional[str] = None,
@@ -173,7 +179,7 @@ def __init__(
         event_logger: Optional[EventLogger] = None,
     ):
         self._decider_context = decider_context
-        self._internal = internal
+        self._internal: RustDecider = internal
         self._span = server_span
         self._context_name = context_name
         if event_logger:
@@ -696,28 +702,6 @@ def get_all_variants_for_identifier_without_expose(
 
         return parsed_choices
 
-    def _get_dynamic_config_value(
-        self,
-        feature_name: str,
-        decider_func: Callable[[str, DeciderContext], Any],
-        default: Any,
-    ) -> Optional[Any]:
-        ctx = self._get_ctx()
-        ctx_err = ctx.err()
-        if ctx_err is not None:
-            logger.info(f"Encountered error in rust_decider.make_ctx(): {ctx_err}")
-            return None
-
-        res = decider_func(feature_name, ctx)
-        if res is None:
-            return default
-        error = res.err()
-        if error:
-            logger.warning(f"Encountered error {decider_func.__name__}: {error}")
-            return default
-
-        return res.val()
-
     def get_bool(self, feature_name: str, default: bool = False) -> bool:
         """Fetch a Dynamic Configuration of boolean type.
 
@@ -728,10 +712,7 @@ def get_bool(self, feature_name: str, default: bool = False) -> bool:
 
         :return: the boolean value of the dyanimc config if it is active/exists, :code:`default` parameter otherwise.
         """
-        decider = self._get_decider()
-        if not decider:
-            return default
-        return self._get_dynamic_config_value(feature_name, decider.get_bool, default)
+        return self._get_dynamic_config_value(feature_name, default, bool, self._internal.get_bool)
 
     def get_int(self, feature_name: str, default: int = 0) -> int:
         """Fetch a Dynamic Configuration of int type.
@@ -743,10 +724,7 @@ def get_int(self, feature_name: str, default: int = 0) -> int:
 
         :return: the int value of the dyanimc config if it is active/exists, :code:`default` parameter otherwise.
         """
-        decider = self._get_decider()
-        if not decider:
-            return default
-        return self._get_dynamic_config_value(feature_name, decider.get_int, default)
+        return self._get_dynamic_config_value(feature_name, default, int, self._internal.get_int)
 
     def get_float(self, feature_name: str, default: float = 0.0) -> float:
         """Fetch a Dynamic Configuration of float type.
@@ -758,10 +736,9 @@ def get_float(self, feature_name: str, default: float = 0.0) -> float:
 
         :return: the float value of the dyanimc config if it is active/exists, :code:`default` parameter otherwise.
         """
-        decider = self._get_decider()
-        if not decider:
-            return default
-        return self._get_dynamic_config_value(feature_name, decider.get_float, default)
+        return self._get_dynamic_config_value(
+            feature_name, default, float, self._internal.get_float
+        )
 
     def get_string(self, feature_name: str, default: str = "") -> str:
         """Fetch a Dynamic Configuration of string type.
@@ -773,10 +750,7 @@ def get_string(self, feature_name: str, default: str = "") -> str:
 
         :return: the string value of the dyanimc config if it is active/exists, :code:`default` parameter otherwise.
         """
-        decider = self._get_decider()
-        if not decider:
-            return default
-        return self._get_dynamic_config_value(feature_name, decider.get_string, default)
+        return self._get_dynamic_config_value(feature_name, default, str, self._internal.get_string)
 
     def get_map(self, feature_name: str, default: Optional[dict] = None) -> Optional[dict]:
         """Fetch a Dynamic Configuration of map type.
@@ -788,10 +762,7 @@ def get_map(self, feature_name: str, default: Optional[dict] = None) -> Optional
 
         :return: the map value of the dyanimc config if it is active/exists, :code:`default` parameter otherwise.
         """
-        decider = self._get_decider()
-        if not decider:
-            return default
-        return self._get_dynamic_config_value(feature_name, decider.get_map, default)
+        return self._get_dynamic_config_value(feature_name, default, dict, self._internal.get_map)
 
     def get_all_dynamic_configs(self) -> List[Dict[str, Any]]:
         """Return a list of dynamic configuration dicts in this format:
@@ -826,40 +797,61 @@ def get_all_dynamic_configs(self) -> List[Dict[str, Any]]:
 
         :return: list of all active dynamic config dicts.
         """
-        decider = self._get_decider()
-        if not decider:
-            return []
-
-        ctx = self._get_ctx()
-        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("rs_decider is None--did not initialize.")
             return []
 
-        all_decisions_result = decider.get_all_values(ctx)
+        ctx = self._decider_context.to_dict()
 
-        error = all_decisions_result.err()
-        if error:
-            logger.info(f"Encountered error in decider.choose_all(): {error}")
+        try:
+            values = self._internal.all_values(ctx)
+        except DeciderException as exc:
+            logger.info(str(exc))
             return []
 
-        all_decisions = all_decisions_result.decisions()
         parsed_configs = []
 
-        for dc_name, decision in all_decisions.items():
-            decision_error = decision.err()
-            if decision_error:
-                logger.info(
-                    f"Encountered error for dynamic config: {dc_name} in decider.get_all_values(): {decision_error}"
-                )
-                continue
+        for feature_name, val in values.items():
+            parsed_configs.append(self._value_to_dc_dict(feature_name, val))
+
+        return parsed_configs
 
-            value_dict = decision.value_dict()
+    def _get_dynamic_config_value(
+        self,
+        feature_name: str,
+        default: Any,
+        dc_type: Type[T],
+        get_fn: Callable[..., Type[T]],
+    ) -> T:
+        if self._internal is None:
+            logger.error("rs_decider is None--did not initialize.")
+            return default
 
-            if value_dict:
-                parsed_configs.append(value_dict)
+        ctx = self._decider_context.to_dict()
 
-        return parsed_configs
+        try:
+            value = get_fn(feature_name=feature_name, context=ctx)
+        except FeatureNotFoundException as exc:
+            warnings.warn(str(exc))
+            return default
+        except ValueTypeMismatchException as exc:
+            logger.info(str(exc))
+            return default
+        except DeciderException as exc:
+            logger.info(str(exc))
+            return default
+
+        try:
+            return dc_type(value)  # type: ignore [call-arg]
+        except TypeError:
+            return default
+
+    def _value_to_dc_dict(self, feature_name: str, value: Optional[Any]) -> Dict[str, Any]:
+        return {
+            "name": feature_name,
+            "value": value,
+            "type": "" if value is None else TYPE_STR_LOOKUP[type(value)],
+        }
 
     def get_experiment(self, experiment_name: str) -> Optional[ExperimentConfig]:
         """Get an :py:class:`~reddit_decider.ExperimentConfig` `dataclass <https://github.com/reddit/experiments.py/blob/develop/reddit_decider/__init__.py#L44>`_

requirements.txt

@@ -1,7 +1,7 @@
 -r requirements-transitive.txt
 baseplate==2.0.0a1
 black==21.4b2
-reddit-decider==1.2.29
+reddit-decider==1.2.30
 flake8==3.9.1
 mypy==0.790
 pyramid==2.0   # required for `from baseplate.frameworks.pyramid import BaseplateRequest` which calls `import pyramid.events`

setup.py

@@ -19,7 +19,7 @@
     install_requires=[
         "baseplate>=2.0.0a1,<3.0",
         "reddit-edgecontext>=1.0.0a3,<2.0",
-        "reddit-decider~=1.2.29",
+        "reddit-decider~=1.2.30",
         "typing_extensions>=3.10.0.0,<5.0",
     ],
     package_data={"reddit_experiments": ["py.typed"]},

tests/decider_tests.py

@@ -1696,11 +1696,10 @@ def test_get_all_values(self):
                 {"name": "dc_missing_map", "value": {}, "type": "map"},
             )
 
-            # set "type" to empty string if "value_type" is missing on cfg
             missing_map_val_res = first_occurrence_of_key_in(
                 configs, "name", "dc_missing_value_type"
             )
             self.assertEqual(
                 missing_map_val_res,
-                {"name": "dc_missing_value_type", "value": False, "type": ""},
+                {"name": "dc_missing_value_type", "value": False, "type": "boolean"},
             )