Merge pull request #7 from mathspace/remove-target

Remove references to target_id and simplify flag evaluation signatures

Author: msmans

src/feafea/__init__.py

@@ -38,9 +38,9 @@ def _hash_percent(s: str, seed: str = "") -> float:
 
     This is not the most efficient hash function but it's stable.
     Stability of this hash function is CRUCIAL. It's used to hash feature flag
-    names and target IDs to ensure consistent evaluation of feature flags across
+    names and split targets to ensure consistent evaluation of feature flags across
     time, different instances, different python versions, operating systems and
-    so on because the distribution of target IDs depend on it.
+    so on because the distribution of split targets depend on it.
     """
     return (
         int.from_bytes(
@@ -361,7 +361,7 @@ class _CompiledFilter:
     __slots__ = ("eval", "flag_refs", "rule_refs")
 
     # The compiled filter function.
-    eval: Callable[[TargetID, Attributes], bool]
+    eval: Callable[[Attributes], bool]
     # The set of flag names referenced in the filter.
     flag_refs: set[str]
     # The set of rule names referenced in the filter.
@@ -564,7 +564,7 @@ def _pythonize(self, f: _ParsedFilter, defined_flags: dict[str, CompiledFlag], d
                         # This will therefore never raise an exception.
                         if self._ignore_undefined_refs and sym_name not in defined_flags:
                             return "False"
-                        lhs = f"flags[{sym_name!r}].eval(target_id, attributes).variant"
+                        lhs = f"flags[{sym_name!r}].eval(attributes).variant"
                         if op in {"IN", "NOTIN"}:
                             return f"{lhs} {self._py_op_map[op]} sets[{arg2!r}]"
                         else:
@@ -598,7 +598,7 @@ def _pythonize(self, f: _ParsedFilter, defined_flags: dict[str, CompiledFlag], d
                         # exists. This will therefore never raise an exception.
                         if self._ignore_undefined_refs and sym_name not in defined_rules:
                             return "False"
-                        lhs = f"rules[{sym_name!r}].eval(target_id, attributes)"
+                        lhs = f"rules[{sym_name!r}].eval(attributes)"
                         return f"{lhs} {self._py_op_map[op]} {arg2!r}"
                     case _:  # pragma: no cover
                         assert False, "unreachable"  # pragma: no cover
@@ -622,7 +622,7 @@ def compile(self, name: str, flags: dict[str, CompiledFlag], rules: dict[str, _C
         optimized = self._optimize(inlined)
         py_expr = self._pythonize(optimized, flags, rules)
         code_str = f"""
-def a(target_id, attributes):
+def a(attributes):
     return {py_expr}
 """
         py_code = compile(code_str, "", "exec")
@@ -645,9 +645,6 @@ def a(target_id, attributes):
     _config_schema = json.load(f)
 
 
-type TargetID = str
-
-
 class _CompiledRule:
     """
     Flag independent compiled rule that evaluates to a boolean indicating
@@ -656,7 +653,7 @@ class _CompiledRule:
 
     __slots__ = ("name", "eval", "_compiled_filter")
     name: str
-    eval: Callable[[TargetID, Attributes], bool]
+    eval: Callable[[Attributes], bool]
     _compiled_filter: _CompiledFilter
 
 
@@ -668,7 +665,7 @@ class _CompiledFlagRule:
 
     __slots__ = ("rule_name", "eval")
     rule_name: str
-    eval: Callable[[TargetID, Attributes], Variant | None]
+    eval: Callable[[Attributes], Variant | None]
 
 
 class CompiledFlag:
@@ -691,13 +688,12 @@ class CompiledFlag:
     _all_rules: dict[str, _CompiledRule]
     _all_flags: dict[str, CompiledFlag]
 
-    def eval(self, target_id: TargetID, attributes: Attributes) -> FlagEvaluation:
+    def eval(self, attributes: Attributes) -> FlagEvaluation:
         e = FlagEvaluation()
         e.default = self.default
         e.flag = self.name
-        e.target_id = target_id
         for rule in self._rules:
-            v = rule.eval(target_id, attributes)
+            v = rule.eval(attributes)
             if v is not None:
                 e.rule = rule.rule_name
                 e.variant = v
@@ -861,11 +857,11 @@ def from_dict(c: DictConfig, ignore_undefined_refs: bool = False) -> CompiledCon
                 compiled_rule._compiled_filter = cf
                 globals["match"] = cf.eval
                 py_common += [f"attributes['__seed'] = {r.get('split_group', rule_name)!r}"]
-                py_common += ["if not match(target_id, attributes): return None"]
+                py_common += ["if not match(attributes): return None"]
 
             # Compile the flag independent rule.
 
-            lines = ["def a(target_id, attributes):"]
+            lines = ["def a(attributes):"]
             for line in py_common:
                 line = re.sub(r"return None$", "return False", line)
                 lines.append(f"  {line}")
@@ -889,7 +885,7 @@ def from_dict(c: DictConfig, ignore_undefined_refs: bool = False) -> CompiledCon
                 # Returns variant | None
                 # - variant: The variant evaluated
                 # - None: The rule does not apply
-                lines = ["def a(target_id, attributes):"]
+                lines = ["def a(attributes):"]
                 lines += list(f"  {line}" for line in (py_common + py_lines))
                 lines += ["  return None"]
                 lines_str = "\n".join(lines)
@@ -972,15 +968,15 @@ class FlagEvaluation:
         "flag",
         "variant",
         "default",
-        "target_id",
+        "attributes",
         "rule",
         "reason",
         "timestamp",
     )
     flag: str
     variant: Variant
     default: Variant
-    target_id: str
+    attributes: Attributes
     rule: str | Literal[""]
     reason: Literal["const_rule", "default"]
     timestamp: float
@@ -1042,14 +1038,12 @@ def _record_eval_metrics(self, e: FlagEvaluation, dur: float):
         }
         _prom_eval_duration.labels(**labels).observe(dur)
 
-    def detailed_evaluate_all(self, config: CompiledConfig, names: Iterable[str], id: str, attributes: Attributes = {}) -> dict[str, FlagEvaluation]:
+    def detailed_evaluate_all(self, config: CompiledConfig, names: Iterable[str], attributes: Attributes = {}) -> dict[str, FlagEvaluation]:
         """
         Evaluate all flags for the given id and attributes and return
         FlagEvaluations. detailed_evaluate_all is thread-safe.
         """
         assert config._feafea_checksum == _feafea_checksum, "incompatible config"
-        if not isinstance(id, str):
-            raise TypeError(f"id must be a string, not {type(id).__name__}")
         self._validate_attributes_type(attributes)
         merged_attributes = {"__now": time.time(), **attributes}
         now = merged_attributes["__now"]
@@ -1062,7 +1056,8 @@ def detailed_evaluate_all(self, config: CompiledConfig, names: Iterable[str], id
             if flag is None:
                 raise ValueError(f"Flag {name} does not exist in the config")
             start = time.perf_counter()
-            e = flag.eval(id, merged_attributes)
+            e = flag.eval(merged_attributes)
+            e.attributes = merged_attributes
             e.timestamp = now
             dur = time.perf_counter() - start
             self._record_eval_metrics(e, dur)
@@ -1088,19 +1083,18 @@ def pop_evaluations(self) -> list[FlagEvaluation]:
             self._evaluations = []
             return es
 
-    def evaluate(self, config: CompiledConfig, name: str, id: str, attributes: Attributes = {}) -> Variant:
+    def evaluate(self, config: CompiledConfig, name: str, attributes: Attributes = {}) -> Variant:
         """
         Evaluate the given flag. evaluate is thread-safe.
 
         name: The name of the flag.
-        id: The id of the flag.
         attributes: The attributes to evaluate the flag with.
         """
-        return next(iter(self.detailed_evaluate_all(config, [name], id, attributes).values())).variant
+        return next(iter(self.detailed_evaluate_all(config, [name], attributes).values())).variant
 
-    def evaluate_all(self, config: CompiledConfig, names: Iterable[str], id: str, attributes: Attributes = {}) -> dict[str, Variant]:
+    def evaluate_all(self, config: CompiledConfig, names: Iterable[str], attributes: Attributes = {}) -> dict[str, Variant]:
         """
         Evaluate all flags for the given id and attributes. evaluate_all is
         thread-safe.
         """
-        return {n: e.variant for n, e in self.detailed_evaluate_all(config, names, id, attributes).items()}
+        return {n: e.variant for n, e in self.detailed_evaluate_all(config, names, attributes).items()}

tests/test_attribute_only_filters.py

@@ -99,7 +99,7 @@ def test_simple(self):
                 fs = _FilterSet()
                 fs.parse("f", filter)
                 c = fs.compile("f", {}, {})
-                self.assertEqual(c.eval("", attr), expected)
+                self.assertEqual(c.eval(attr), expected)
 
         valid_syntax_cases = [
             "flag:a > 3",
@@ -208,12 +208,12 @@ def test_invalid_attributes(self):
                 fs = _FilterSet()
                 fs.parse("f", f"({filter}) and attr:true")
                 cf = fs.compile("f", {}, {})
-                self.assertFalse(cf.eval("", {**attrs, "true": True}))
+                self.assertFalse(cf.eval({**attrs, "true": True}))
 
                 fs = _FilterSet()
                 fs.parse("f", f"({filter}) or attr:true")
                 cf = fs.compile("f", {}, {})
-                self.assertTrue(cf.eval("", {**attrs, "true": True}))
+                self.assertTrue(cf.eval({**attrs, "true": True}))
 
     def test_invalid_symbol_names(self):
         cases = [
@@ -287,7 +287,7 @@ def test_inlined_filters(self):
         for filter, attr, expected in cases:
             with self.subTest(f"{filter}, {attr}"):
                 c = fs.compile(filter, {}, {})
-                self.assertEqual(c.eval("", attr), expected)
+                self.assertEqual(c.eval(attr), expected)
 
     def test_missing_ref_filter(self):
         fs = _FilterSet()
@@ -314,11 +314,11 @@ def test_insplit_function_distribution_and_exclusivity(self):
 
         variant_count = [0, 0, 0]
         for id in range(100000):
-            if filter_a.eval("", {"a": id}):
+            if filter_a.eval({"a": id}):
                 variant_count[0] += 1
-            if filter_b.eval("", {"a": id}):
+            if filter_b.eval({"a": id}):
                 variant_count[1] += 1
-            if filter_c.eval("", {"a": id}):
+            if filter_c.eval({"a": id}):
                 variant_count[2] += 1
 
         self.assertEqual(sum(variant_count), 100000)
@@ -338,7 +338,7 @@ def test_insplit_function_seed_spread(self):
             # Here, we keep the attribute value constant and vary the seed and
             # we expect the same distribution as if we were varying the
             # attribute value.
-            if filter.eval("", {"a": "constant", "__seed": id}):
+            if filter.eval({"a": "constant", "__seed": id}):
                 true_count += 1
             else:
                 false_count += 1

tests/test_attribute_only_filters_undefined_refs.py

@@ -101,7 +101,7 @@ def test_simple(self):
                 fs = _FilterSet()
                 fs.parse("f", filter)
                 c = fs.compile("f", {}, {})
-                self.assertEqual(c.eval("", attr), expected)
+                self.assertEqual(c.eval(attr), expected)
 
         valid_syntax_cases = [
             "flag:a > 3",
@@ -182,12 +182,12 @@ def test_invalid_attributes(self):
                 fs = _FilterSet()
                 fs.parse("f", f"({filter}) and attr:true")
                 cf = fs.compile("f", {}, {})
-                self.assertFalse(cf.eval("", {**attrs, "true": True}))
+                self.assertFalse(cf.eval({**attrs, "true": True}))
 
                 fs = _FilterSet()
                 fs.parse("f", f"({filter}) or attr:true")
                 cf = fs.compile("f", {}, {})
-                self.assertTrue(cf.eval("", {**attrs, "true": True}))
+                self.assertTrue(cf.eval({**attrs, "true": True}))
 
     def test_invalid_symbol_names(self):
         cases = [
@@ -261,7 +261,7 @@ def test_inlined_filters(self):
         for filter, attr, expected in cases:
             with self.subTest(f"{filter}, {attr}"):
                 c = fs.compile(filter, {}, {})
-                self.assertEqual(c.eval("", attr), expected)
+                self.assertEqual(c.eval(attr), expected)
 
     def test_missing_ref_filter(self):
         fs = _FilterSet()
@@ -270,11 +270,11 @@ def test_missing_ref_filter(self):
         fs.parse("d", "filter:c or filter:e")
         fs.parse("e", "attr:age > 10")
         c = fs.compile("a", {}, {})
-        self.assertFalse(c.eval("", {}))
+        self.assertFalse(c.eval({}))
         c = fs.compile("c", {}, {})
-        self.assertFalse(c.eval("", {}))
+        self.assertFalse(c.eval({}))
         c = fs.compile("e", {}, {})
-        self.assertTrue(c.eval("", {"age": 20}))
+        self.assertTrue(c.eval({"age": 20}))
 
     def test_rule_and_flag_refs(self):
         fs = _FilterSet()
@@ -295,11 +295,11 @@ def test_insplit_function_distribution_and_exclusivity(self):
 
         variant_count = [0, 0, 0]
         for id in range(100000):
-            if filter_a.eval("", {"a": id}):
+            if filter_a.eval({"a": id}):
                 variant_count[0] += 1
-            if filter_b.eval("", {"a": id}):
+            if filter_b.eval({"a": id}):
                 variant_count[1] += 1
-            if filter_c.eval("", {"a": id}):
+            if filter_c.eval({"a": id}):
                 variant_count[2] += 1
 
         self.assertEqual(sum(variant_count), 100000)
@@ -319,7 +319,7 @@ def test_insplit_function_seed_spread(self):
             # Here, we keep the attribute value constant and vary the seed and
             # we expect the same distribution as if we were varying the
             # attribute value.
-            if filter.eval("", {"a": "constant", "__seed": id}):
+            if filter.eval({"a": "constant", "__seed": id}):
                 true_count += 1
             else:
                 false_count += 1
@@ -346,7 +346,7 @@ def test_undefined_flag_reference_in_pythonize(self):
                 fs = _FilterSet()
                 fs.parse("f", filter_str)
                 c = fs.compile("f", {}, {})  # Empty flags dict means undefined
-                self.assertEqual(c.eval("", attr), expected)
+                self.assertEqual(c.eval(attr), expected)
 
     def test_undefined_rule_reference_in_pythonize(self):
         """Test that undefined rule references return False when ignore_undefined_refs is True."""
@@ -362,4 +362,4 @@ def test_undefined_rule_reference_in_pythonize(self):
                 fs = _FilterSet()
                 fs.parse("f", filter_str)
                 c = fs.compile("f", {}, {})  # Empty rules dict means undefined
-                self.assertEqual(c.eval("", attr), expected)
+                self.assertEqual(c.eval(attr), expected)

tests/test_configs.py

@@ -207,7 +207,7 @@ def test_config_serialization(self):
         b = self._valid_config.to_bytes()
         cc = CompiledConfig.from_bytes(b)
         self.assertSetEqual(set(self._valid_config.flags), set(cc.flags))
-        self.assertEqual(self._valid_config.flags["a"].eval("", {}).variant, cc.flags["a"].eval("", {}).variant)
+        self.assertEqual(self._valid_config.flags["a"].eval({}).variant, cc.flags["a"].eval({}).variant)
 
     def test_valid_const_config(self):
         cases = [
@@ -233,7 +233,7 @@ def test_valid_const_config(self):
 
         for flag, attrs, reason, rule, variant in cases:
             with self.subTest(f"flag={flag}, attrs={attrs}"):
-                ev = self._valid_config.flags[flag].eval("", attrs)
+                ev = self._valid_config.flags[flag].eval(attrs)
                 self.assertEqual(ev.variant, variant)
                 self.assertEqual(ev.reason, reason)
                 self.assertEqual(ev.rule, rule)

tests/test_configs_undefined_refs.py

@@ -152,7 +152,7 @@ def test_config_serialization(self):
         b = self._valid_config.to_bytes()
         cc = CompiledConfig.from_bytes(b)
         self.assertSetEqual(set(self._valid_config.flags), set(cc.flags))
-        self.assertEqual(self._valid_config.flags["a"].eval("", {}).variant, cc.flags["a"].eval("", {}).variant)
+        self.assertEqual(self._valid_config.flags["a"].eval({}).variant, cc.flags["a"].eval({}).variant)
 
     def test_valid_const_config(self):
         cases = [
@@ -178,7 +178,7 @@ def test_valid_const_config(self):
 
         for flag, attrs, reason, rule, variant in cases:
             with self.subTest(f"flag={flag}, attrs={attrs}"):
-                ev = self._valid_config.flags[flag].eval("", attrs)
+                ev = self._valid_config.flags[flag].eval(attrs)
                 self.assertEqual(ev.variant, variant)
                 self.assertEqual(ev.reason, reason)
                 self.assertEqual(ev.rule, rule)