Merge pull request #109 from reddit/update_docs_w_version

Bump Decider (re-enable `PartialLoad` error), update version in docs + tests

Author: mrlevitas

docs/index.rst

@@ -43,7 +43,9 @@ Setup :code:`reddit-experiments` in your application's configuration file:
    experiments.path = /var/local/foo.json
 
    # optional: how long to wait for the experiments file to exist before failing
-   # (default: do not wait. fail immediately if not available)
+   # default:
+   #    >= v1.7.0 wait 30 seconds
+   #    <  v1.7.0 do not wait, fail immediately if not available
    experiments.timeout = 60 seconds
 
    # optional: the base amount of time for exponential backoff while waiting
@@ -62,7 +64,7 @@ Upgrade or integrate reddit-experiments package:
 .. code-block:: python
 
     # import latest reddit-experiments package in service requirements.txt
-    reddit-experiments>=1.4.0
+    reddit-experiments>=1.7.0
 
 Initialize :code:`decider` instance on Baseplate context
 --------------------------------------------------------

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

requirements.txt

@@ -1,7 +1,7 @@
 -r requirements-transitive.txt
 baseplate>=2.0.0a1,<3.0
 black==21.4b2
-reddit-decider==1.3.1
+reddit-decider==1.4.1
 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.3.1",
+        "reddit-decider~=1.4.1",
         "typing_extensions>=3.10.0.0,<5.0",
     ],
     package_data={"reddit_experiments": ["py.typed"], "reddit_decider": ["py.typed"]},

tests/decider_tests.py

@@ -497,7 +497,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': 'invalid type: string \"1\", expected u32'}"
+                    "Partially loaded Decider: 1 features failed to load: {'test': 'Manifest parsing error: invalid type: string \"1\", expected u32'}"
                     in x.getMessage()
                     for x in captured.records
                 )
@@ -1481,6 +1481,59 @@ def test_get_variant_without_expose_with_HG_as_control_1_and_child_returns_none_
                 experiment_name="hg", variant="control_1", event_fields=event_fields
             )
 
+    def test_get_variant_for_okta_groups(self):
+        identifier = "t2_test"
+        bucket_val = "user_id"
+        group_overrides = {"variant_2": {"EQ": {"field": "user_id", "values": ["$some_group_id"]}}}
+
+        self.exp_base_config["exp_1"]["experiment"].update({"overrides": [group_overrides]})
+        # reset variant for override to make sure it's not organically bucketed into it
+        self.exp_base_config["exp_1"]["experiment"].update(
+            {
+                "variants": [
+                    {"range_start": 0.0, "range_end": 0.0, "name": "variant_2"},
+                ]
+            }
+        )
+
+        og_cfg = {
+            "$override_groups": {
+                "id": 1337,
+                "value": {
+                    "$some_group_id": {
+                        "name": "some_group_id",
+                        "values": [identifier],
+                        "field": "user_id",
+                    }
+                },
+                "type": "dynamic_config",
+                "version": "1",
+                "enabled": False,
+                "owner": "test",
+                "name": "$override_group",
+                "value_type": "Map",
+                "experiment": {"experiment_version": 1},
+            },
+        }
+        self.exp_base_config.update(og_cfg)
+
+        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)
+            variant = decider.get_variant_for_identifier_without_expose(
+                experiment_name="exp_1", identifier=identifier, identifier_type=bucket_val
+            )
+            self.assertEqual(variant, "variant_2")
+
+            self.dc._user_id = identifier
+            decider = setup_decider(f, self.dc, self.mock_span, self.event_logger)
+            variant = decider.get_variant_without_expose(experiment_name="exp_1")
+            self.assertEqual(variant, "variant_2")
+
+            # no exposures should be triggered
+            self.assertEqual(self.event_logger.log.call_count, 0)
+
 
 class TestDeciderGetDynamicConfig(unittest.TestCase):
     def setUp(self):
@@ -1574,6 +1627,18 @@ def test_get_map(self):
             res = decider.get_string("dc_1")
             self.assertEqual(res, "")
 
+    def test_get_map_disabled(self):
+        self.dc_base_config["dc_1"].update(
+            {"value_type": "Map", "value": {"key": "value", "another_key": "another_value"}}
+        )
+        self.dc_base_config["dc_1"].update({"enabled": False})
+
+        with create_temp_config_file(self.dc_base_config) as f:
+            decider = setup_decider(f, self.dc, self.mock_span, self.event_logger)
+
+            res = decider.get_map("dc_1")
+            self.assertEqual(res, None)
+
     def test_get_all_values(self):
         base_cfg = self.dc_base_config["dc_1"].copy()
 

tests/experiment_tests.py

@@ -674,6 +674,42 @@ def test_none_returned_on_variant_call_with_no_experiment_with_cfg_data(self):
         variant = experiments.variant("test", user=self.user)
         self.assertEqual(variant, None)
 
+    def test_new_identifier_compatibility(self):
+        self.mock_filewatcher.get_data.return_value = {
+            "test": {
+                "id": 1,
+                "name": "test",
+                "enabled": True,
+                "owner": "test_owner",
+                "version": "1",
+                "emit_event": True,
+                "type": "range_variant",
+                "start_ts": time.time() - THIRTY_DAYS,
+                "stop_ts": time.time() + THIRTY_DAYS,
+                "experiment": {
+                    "variants": [
+                        {"name": "active", "size": 1, "range_end": 1.0, "range_start": 0},
+                    ],
+                    "experiment_version": 5,
+                    "shuffle_version": 91,
+                    "bucket_val": "ad_account_id",
+                    "log_bucketing": False,
+                },
+            }
+        }
+
+        experiments = Experiments(
+            config_watcher=self.mock_filewatcher,
+            server_span=self.mock_span,
+            context_name="test",
+            event_logger=self.event_logger,
+        )
+
+        self.assertEqual(self.event_logger.log.call_count, 0)
+        variant = experiments.variant("test", ad_account_id="a2_xxx")
+
+        self.assertEqual(variant, "active")
+
 
 @mock.patch("reddit_experiments.FileWatcher")
 class ExperimentsClientFromConfigTests(unittest.TestCase):