Move decider_client_from_config to reddit_experiments

reddit_decider/__init__.py

@@ -1,17 +1,18 @@
 import logging
 
-from typing import Any, Callable, Dict, Optional
+from typing import Any, Callable, Dict, IO, Optional
 
+from baseplate import RequestContext
 from baseplate import Span
 from baseplate.clients import ContextFactory
-from baseplate.frameworks.pyramid import BaseplateRequest
 from baseplate.lib import config
 from baseplate.lib.events import DebugLogger
 from baseplate.lib.events import EventLogger
 from baseplate.lib.file_watcher import FileWatcher
+from baseplate.lib.file_watcher import T
 from baseplate.lib.file_watcher import WatchedFileNotAvailableError
 
-import rust_decider
+import rust_decider # type: ignore
 
 
 logger = logging.getLogger(__name__)
@@ -69,7 +70,7 @@ def to_dict(self) -> Dict:
         }
 
 
-def init_decider_parser(file):
+def init_decider_parser(file: IO) -> Any:
     return rust_decider.init("darkmode overrides targeting fractional_availability value", file.name)
 
 def validate_decider(decider: Optional[Any]) -> None:
@@ -108,7 +109,7 @@ def __init__(
         else:
             self._event_logger = DebugLogger()
 
-    def _get_decider(self):
+    def _get_decider(self) -> Optional[T]:
         try:
             decider = self._config_watcher.get_data()
             validate_decider(decider)
@@ -142,6 +143,9 @@ def get_variant(
         :return: Variant name if a variant is assigned, None otherwise.
         """
         decider = self._get_decider()
+        if not decider:
+            logger.warning("Encountered error in _get_decider()")
+            return None
 
         ctx = rust_decider.make_ctx(self._decider_context.to_dict())
         ctx_err = ctx.err()
@@ -223,7 +227,7 @@ def __init__(
         event_logger: Optional[EventLogger] = None,
         timeout: Optional[float] = None,
         backoff: Optional[float] = None,
-        request_field_extractor: Callable[[BaseplateRequest], Dict[str, str]] = None
+        request_field_extractor: Optional[Callable[[RequestContext], Dict[str, str]]] = None
     ):
         self._filewatcher = FileWatcher(path=path, parser=init_decider_parser, timeout=timeout, backoff=backoff)
         self._event_logger = event_logger
@@ -283,69 +287,3 @@ def make_object_for_context(self, name: str, span: Span) -> Decider:
             context_name=name,
             event_logger=self._event_logger,
         )
-
-
-def decider_client_from_config(
-    app_config: config.RawConfig,
-    event_logger: EventLogger,
-    prefix: str = "experiments.",
-    request_field_extractor: Callable[[BaseplateRequest], Dict[str, str]] = None
-) -> DeciderContextFactory:
-    """Configure and return an :py:class:`DeciderContextFactory` object.
-
-    The keys useful to :py:func:`decider_client_from_config` should be prefixed, e.g.
-    ``experiments.path``, etc.
-
-    Supported keys:
-
-    ``path`` (optional)
-        The path to the experiment configuration file generated by the
-        experiment configuration fetcher daemon.
-    ``timeout`` (optional)
-        The time that we should wait for the file specified by ``path`` to
-        exist.  Defaults to `None` which is `infinite`.
-    ``backoff`` (optional)
-        The base amount of time for exponential backoff when trying to find the
-        experiments config file. Defaults to no backoff between tries.
-    ``request_field_extractor`` (optional) used to populate
-        "app_name" & "build_number" fields in DeciderContext()
-
-    :param raw_config: The application configuration which should have
-        settings for the experiments client.
-    :param event_logger: The EventLogger to be used to log bucketing events.
-    :param prefix: the prefix used to filter keys (defaults to "experiments.").
-
-    """
-    assert prefix.endswith(".")
-    config_prefix = prefix[:-1]
-
-    cfg = config.parse_config(
-        app_config,
-        {
-            config_prefix: {
-                "path": config.Optional(config.String, default="/var/local/experiments.json"),
-                "timeout": config.Optional(config.Timespan),
-                "backoff": config.Optional(config.Timespan),
-            }
-        },
-    )
-    options = getattr(cfg, config_prefix)
-
-    # pylint: disable=maybe-no-member
-    if options.timeout:
-        timeout = options.timeout.total_seconds()
-    else:
-        timeout = None
-
-    if options.backoff:
-        backoff = options.backoff.total_seconds()
-    else:
-        backoff = None
-
-    return DeciderContextFactory(
-        path=options.path,
-        event_logger=event_logger,
-        timeout=timeout,
-        backoff=backoff,
-        request_field_extractor=request_field_extractor
-    )

reddit_experiments/__init__.py

@@ -3,12 +3,14 @@
 import warnings
 
 from enum import Enum
+from typing import Callable
 from typing import Dict
 from typing import Optional
 from typing import overload
 from typing import Sequence
 from typing import Set
 
+from baseplate import RequestContext
 from baseplate import Span
 from baseplate.clients import ContextFactory
 from baseplate.lib import config
@@ -19,6 +21,7 @@
 from baseplate.lib.file_watcher import WatchedFileNotAvailableError
 from reddit_edgecontext import User
 
+from reddit_decider import DeciderContextFactory
 from reddit_experiments.providers import parse_experiment
 from reddit_experiments.providers.base import Experiment
 
@@ -396,3 +399,69 @@ def experiments_client_from_config(
         backoff = None
 
     return ExperimentsContextFactory(options.path, event_logger, timeout=timeout, backoff=backoff)
+
+
+def decider_client_from_config(
+    app_config: config.RawConfig,
+    event_logger: EventLogger,
+    prefix: str = "experiments.",
+    request_field_extractor: Optional[Callable[[RequestContext], Dict[str, str]]] = None,
+) -> DeciderContextFactory:
+    """Configure and return an :py:class:`DeciderContextFactory` object.
+
+    The keys useful to :py:func:`decider_client_from_config` should be prefixed, e.g.
+    ``experiments.path``, etc.
+
+    Supported keys:
+
+    ``path`` (optional)
+        The path to the experiment configuration file generated by the
+        experiment configuration fetcher daemon.
+    ``timeout`` (optional)
+        The time that we should wait for the file specified by ``path`` to
+        exist.  Defaults to `None` which is `infinite`.
+    ``backoff`` (optional)
+        The base amount of time for exponential backoff when trying to find the
+        experiments config file. Defaults to no backoff between tries.
+    ``request_field_extractor`` (optional) used to populate
+        "app_name" & "build_number" fields in DeciderContext()
+
+    :param raw_config: The application configuration which should have
+        settings for the experiments client.
+    :param event_logger: The EventLogger to be used to log bucketing events.
+    :param prefix: the prefix used to filter keys (defaults to "experiments.").
+
+    """
+    assert prefix.endswith(".")
+    config_prefix = prefix[:-1]
+
+    cfg = config.parse_config(
+        app_config,
+        {
+            config_prefix: {
+                "path": config.Optional(config.String, default="/var/local/experiments.json"),
+                "timeout": config.Optional(config.Timespan),
+                "backoff": config.Optional(config.Timespan),
+            }
+        },
+    )
+    options = getattr(cfg, config_prefix)
+
+    # pylint: disable=maybe-no-member
+    if options.timeout:
+        timeout = options.timeout.total_seconds()
+    else:
+        timeout = None
+
+    if options.backoff:
+        backoff = options.backoff.total_seconds()
+    else:
+        backoff = None
+
+    return DeciderContextFactory(
+        path=options.path,
+        event_logger=event_logger,
+        timeout=timeout,
+        backoff=backoff,
+        request_field_extractor=request_field_extractor,
+    )

tests/decider_tests.py

@@ -12,49 +12,10 @@
 from reddit_edgecontext import User
 
 from reddit_decider import Decider
-from reddit_decider import decider_client_from_config
 from reddit_decider import DeciderContext
 from reddit_decider import DeciderContextFactory
 from reddit_decider import init_decider_parser
-
-
-@mock.patch("reddit_decider.FileWatcher")
-class DeciderClientFromConfigTests(unittest.TestCase):
-    def setUp(self):
-        super().setUp()
-        self.event_logger = mock.Mock(spec=DebugLogger)
-        self.mock_span = mock.MagicMock(spec=ServerSpan)
-        self.mock_span.context = None
-
-    def test_make_clients(self, file_watcher_mock):
-        decider_ctx_factory = decider_client_from_config(
-            {"experiments.path": "/tmp/test"}, self.event_logger
-        )
-        self.assertIsInstance(decider_ctx_factory, DeciderContextFactory)
-        file_watcher_mock.assert_called_once_with(
-            path="/tmp/test", parser=init_decider_parser, timeout=None, backoff=None
-        )
-
-    def test_timeout(self, file_watcher_mock):
-        decider_ctx_factory = decider_client_from_config(
-            {"experiments.path": "/tmp/test", "experiments.timeout": "60 seconds"},
-            self.event_logger,
-        )
-        self.assertIsInstance(decider_ctx_factory, DeciderContextFactory)
-        file_watcher_mock.assert_called_once_with(
-            path="/tmp/test", parser=init_decider_parser, timeout=60.0, backoff=None
-        )
-
-    def test_prefix(self, file_watcher_mock):
-        decider_ctx_factory = decider_client_from_config(
-            {"r2_experiments.path": "/tmp/test", "r2_experiments.timeout": "60 seconds"},
-            self.event_logger,
-            prefix="r2_experiments.",
-        )
-        self.assertIsInstance(decider_ctx_factory, DeciderContextFactory)
-        file_watcher_mock.assert_called_once_with(
-            path="/tmp/test", parser=init_decider_parser, timeout=60.0, backoff=None
-        )
+from reddit_experiments import decider_client_from_config
 
 
 @mock.patch("reddit_decider.FileWatcher")

tests/experiment_tests.py

@@ -12,6 +12,9 @@
 from reddit_edgecontext import AuthenticationToken
 from reddit_edgecontext import User
 
+from reddit_decider import DeciderContextFactory
+from reddit_decider import init_decider_parser
+from reddit_experiments import decider_client_from_config
 from reddit_experiments import EventType
 from reddit_experiments import Experiments
 from reddit_experiments import experiments_client_from_config
@@ -894,3 +897,42 @@ def test_experiments_with_different_name_updated_cache(self):
         self.assertTrue("test2" in self.experiments_factory._global_cache)
         self.assertFalse("test1" in self.experiments_factory._global_cache)
         self.assertEqual(self.experiments_factory._global_cache["test2"].owner, "test2")
+
+
+@mock.patch("reddit_decider.FileWatcher")
+class DeciderClientFromConfigTests(unittest.TestCase):
+    def setUp(self):
+        super().setUp()
+        self.event_logger = mock.Mock(spec=DebugLogger)
+        self.mock_span = mock.MagicMock(spec=ServerSpan)
+        self.mock_span.context = None
+
+    def test_make_clients(self, file_watcher_mock):
+        decider_ctx_factory = decider_client_from_config(
+            {"experiments.path": "/tmp/test"}, self.event_logger
+        )
+        self.assertIsInstance(decider_ctx_factory, DeciderContextFactory)
+        file_watcher_mock.assert_called_once_with(
+            path="/tmp/test", parser=init_decider_parser, timeout=None, backoff=None
+        )
+
+    def test_timeout(self, file_watcher_mock):
+        decider_ctx_factory = decider_client_from_config(
+            {"experiments.path": "/tmp/test", "experiments.timeout": "60 seconds"},
+            self.event_logger,
+        )
+        self.assertIsInstance(decider_ctx_factory, DeciderContextFactory)
+        file_watcher_mock.assert_called_once_with(
+            path="/tmp/test", parser=init_decider_parser, timeout=60.0, backoff=None
+        )
+
+    def test_prefix(self, file_watcher_mock):
+        decider_ctx_factory = decider_client_from_config(
+            {"r2_experiments.path": "/tmp/test", "r2_experiments.timeout": "60 seconds"},
+            self.event_logger,
+            prefix="r2_experiments.",
+        )
+        self.assertIsInstance(decider_ctx_factory, DeciderContextFactory)
+        file_watcher_mock.assert_called_once_with(
+            path="/tmp/test", parser=init_decider_parser, timeout=60.0, backoff=None
+        )