From 39d4a84f220bdb1394c3d95d1d982caf4123259d Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Wed, 19 Jun 2024 19:22:37 +0200 Subject: [PATCH 1/6] feat: Change fractional custom op from percentage-based to relative weighting. Signed-off-by: Simon Schrottner --- .../flagd/resolvers/process/custom_ops.py | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py b/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py index 17763615..2ce89134 100644 --- a/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py +++ b/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py @@ -31,23 +31,35 @@ def fractional(data: dict, *args: JsonLogicArg) -> typing.Optional[str]: logger.error("No hashKey value resolved") return None - hash_ratio = abs(mmh3.hash(bucket_by)) / (2**31 - 1) - bucket = int(hash_ratio * 100) + hash_ratio = abs(mmh3.hash(bucket_by)) / (2 ** 31 - 1) + bucket = hash_ratio * 100 + totalWeight = 0 for arg in args: if ( not isinstance(arg, (tuple, list)) - or len(arg) != 2 - or not isinstance(arg[0], str) - or not isinstance(arg[1], int) + or len(arg) == 0 ): logger.error("Fractional variant weights must be (str, int) tuple") return None + + if (not isinstance(arg[0], str)): + logger.error("Fractional variant's first element isn't string") + return None + if (len(arg) >= 2): + if (not isinstance(arg[1], int)): + logger.error("Fractional variant's second element isn't int") + return None + else: + arg.append(1) + + totalWeight += arg[1] + variant_weights: typing.Tuple[typing.Tuple[str, int]] = args # type: ignore[assignment] range_end = 0 for variant, weight in variant_weights: - range_end += weight + range_end += weight * 100 / totalWeight if bucket < range_end: return variant @@ -69,7 +81,7 @@ def f(s1: str, s2: str) -> bool: def string_comp( - comparator: typing.Callable[[str, str], bool], data: dict, *args: JsonLogicArg + comparator: typing.Callable[[str, str], bool], data: dict, *args: JsonLogicArg ) -> typing.Optional[bool]: if not args: logger.error("No arguments provided to string_comp operator.") From 05a8ccade93dee60abced556c4bf62d320ffce12 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Thu, 20 Jun 2024 13:18:36 +0200 Subject: [PATCH 2/6] review suggestions Signed-off-by: Simon Schrottner --- .../flagd/resolvers/process/custom_ops.py | 68 ++++++++++++------- ...invalid-fractional-args-wrong-content.json | 16 +++++ .../invalid-fractional-weights-strings.json | 19 ++++++ .../tests/test_errors.py | 2 + 4 files changed, 79 insertions(+), 26 deletions(-) create mode 100644 providers/openfeature-provider-flagd/tests/flags/invalid-fractional-args-wrong-content.json create mode 100644 providers/openfeature-provider-flagd/tests/flags/invalid-fractional-weights-strings.json diff --git a/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py b/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py index 2ce89134..e05e2ef0 100644 --- a/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py +++ b/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py @@ -10,6 +10,12 @@ logger = logging.getLogger("openfeature.contrib") +class Fraction: + weight: int = 1 + variant: str + pass + + def fractional(data: dict, *args: JsonLogicArg) -> typing.Optional[str]: if not args: logger.error("No arguments provided to fractional operator.") @@ -31,39 +37,49 @@ def fractional(data: dict, *args: JsonLogicArg) -> typing.Optional[str]: logger.error("No hashKey value resolved") return None - hash_ratio = abs(mmh3.hash(bucket_by)) / (2 ** 31 - 1) + hash_ratio = abs(mmh3.hash(bucket_by)) / (2**31 - 1) bucket = hash_ratio * 100 - totalWeight = 0 + total_weight = 0 + fractions = [] for arg in args: - if ( - not isinstance(arg, (tuple, list)) - or len(arg) == 0 - ): - logger.error("Fractional variant weights must be (str, int) tuple") - return None + fraction = __parse_fraction(arg) + if fraction: + fractions.append(fraction) + total_weight += fraction.weight + + range_end: float = 0 + for fraction in fractions: + range_end += fraction.weight * 100 / total_weight + if bucket < range_end: + return fraction.variant - if (not isinstance(arg[0], str)): - logger.error("Fractional variant's first element isn't string") - return None - if (len(arg) >= 2): - if (not isinstance(arg[1], int)): - logger.error("Fractional variant's second element isn't int") - return None - else: - arg.append(1) + return None - totalWeight += arg[1] - variant_weights: typing.Tuple[typing.Tuple[str, int]] = args # type: ignore[assignment] +def __parse_fraction(arg: JsonLogicArg) -> typing.Optional[Fraction]: + if not isinstance(arg, (tuple, list)) or len(arg) == 0: + logger.error("Fractional variant weights must be (str, int) tuple") + return None - range_end = 0 - for variant, weight in variant_weights: - range_end += weight * 100 / totalWeight - if bucket < range_end: - return variant + if not isinstance(arg[0], str): + logger.error( + "Fractional variant identifier (first element) isn't of type 'str'" + ) + return None - return None + if len(arg) >= 2 and not isinstance(arg[1], int): + logger.error( + "Fractional variant weight value (second element) isn't of type 'int'" + ) + return None + + fraction = Fraction() + if len(arg) >= 2: + fraction.weight = arg[1] + fraction.variant = arg[0] + + return fraction def starts_with(data: dict, *args: JsonLogicArg) -> typing.Optional[bool]: @@ -81,7 +97,7 @@ def f(s1: str, s2: str) -> bool: def string_comp( - comparator: typing.Callable[[str, str], bool], data: dict, *args: JsonLogicArg + comparator: typing.Callable[[str, str], bool], data: dict, *args: JsonLogicArg ) -> typing.Optional[bool]: if not args: logger.error("No arguments provided to string_comp operator.") diff --git a/providers/openfeature-provider-flagd/tests/flags/invalid-fractional-args-wrong-content.json b/providers/openfeature-provider-flagd/tests/flags/invalid-fractional-args-wrong-content.json new file mode 100644 index 00000000..a40e34ba --- /dev/null +++ b/providers/openfeature-provider-flagd/tests/flags/invalid-fractional-args-wrong-content.json @@ -0,0 +1,16 @@ +{ + "flags": { + "basic-flag": { + "state": "ENABLED", + "variants": { + "default": "default", + "true": "true", + "false": "false" + }, + "defaultVariant": "default", + "targeting": { + "fractional": [[]] + } + } + } +} diff --git a/providers/openfeature-provider-flagd/tests/flags/invalid-fractional-weights-strings.json b/providers/openfeature-provider-flagd/tests/flags/invalid-fractional-weights-strings.json new file mode 100644 index 00000000..2f62796e --- /dev/null +++ b/providers/openfeature-provider-flagd/tests/flags/invalid-fractional-weights-strings.json @@ -0,0 +1,19 @@ +{ + "flags": { + "basic-flag": { + "state": "ENABLED", + "variants": { + "default": "default", + "true": "true", + "false": "false" + }, + "defaultVariant": "default", + "targeting": { + "fractional": [ + ["a", "one"], + ["b", "one"] + ] + } + } + } +} diff --git a/providers/openfeature-provider-flagd/tests/test_errors.py b/providers/openfeature-provider-flagd/tests/test_errors.py index 4adb332e..3e576e8a 100644 --- a/providers/openfeature-provider-flagd/tests/test_errors.py +++ b/providers/openfeature-provider-flagd/tests/test_errors.py @@ -48,7 +48,9 @@ def test_file_load_errors(file_name: str): "invalid-semver-args.json", "invalid-stringcomp-args.json", "invalid-fractional-args.json", + "invalid-fractional-args-wrong-content.json", "invalid-fractional-weights.json", + "invalid-fractional-weights-strings.json", ], ) def test_json_logic_parse_errors(file_name: str): From bed926ef6519f8b181f11eefb349cbdc415b629b Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Wed, 26 Jun 2024 08:19:28 +0200 Subject: [PATCH 3/6] Update providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Anton Grübel Signed-off-by: Simon Schrottner --- .../contrib/provider/flagd/resolvers/process/custom_ops.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py b/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py index e05e2ef0..eb2f0dba 100644 --- a/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py +++ b/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py @@ -10,10 +10,10 @@ logger = logging.getLogger("openfeature.contrib") +@dataclass class Fraction: - weight: int = 1 variant: str - pass + weight: int = 1 def fractional(data: dict, *args: JsonLogicArg) -> typing.Optional[str]: From 73374dcbd6a9d499530187274f12c968d7b628a7 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Wed, 26 Jun 2024 08:19:35 +0200 Subject: [PATCH 4/6] Update providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Anton Grübel Signed-off-by: Simon Schrottner --- .../contrib/provider/flagd/resolvers/process/custom_ops.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py b/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py index eb2f0dba..c9161540 100644 --- a/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py +++ b/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py @@ -58,7 +58,7 @@ def fractional(data: dict, *args: JsonLogicArg) -> typing.Optional[str]: def __parse_fraction(arg: JsonLogicArg) -> typing.Optional[Fraction]: - if not isinstance(arg, (tuple, list)) or len(arg) == 0: + if not isinstance(arg, (tuple, list)) or not arg: logger.error("Fractional variant weights must be (str, int) tuple") return None From c0f4a5586471eec2ef7546eaa22c33a40f18ff12 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Wed, 26 Jun 2024 08:24:53 +0200 Subject: [PATCH 5/6] improve log message Signed-off-by: Simon Schrottner --- .../provider/flagd/resolvers/process/custom_ops.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py b/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py index c9161540..c5e3f29d 100644 --- a/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py +++ b/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py @@ -1,5 +1,6 @@ import logging import typing +from dataclasses import dataclass import mmh3 import semver @@ -59,7 +60,9 @@ def fractional(data: dict, *args: JsonLogicArg) -> typing.Optional[str]: def __parse_fraction(arg: JsonLogicArg) -> typing.Optional[Fraction]: if not isinstance(arg, (tuple, list)) or not arg: - logger.error("Fractional variant weights must be (str, int) tuple") + logger.error( + "Fractional variant weights must be (str, int) tuple or [str] list" + ) return None if not isinstance(arg[0], str): @@ -74,10 +77,9 @@ def __parse_fraction(arg: JsonLogicArg) -> typing.Optional[Fraction]: ) return None - fraction = Fraction() + fraction = Fraction(variant=arg[0]) if len(arg) >= 2: fraction.weight = arg[1] - fraction.variant = arg[0] return fraction From 06e7a5c4434d3a652f76053aad008bd5bfda4eac Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Mon, 8 Jul 2024 14:57:27 +0200 Subject: [PATCH 6/6] Update providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py Co-authored-by: Matthew Elwell Signed-off-by: Simon Schrottner --- .../contrib/provider/flagd/resolvers/process/custom_ops.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py b/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py index c5e3f29d..44bb5a7a 100644 --- a/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py +++ b/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py @@ -44,7 +44,7 @@ def fractional(data: dict, *args: JsonLogicArg) -> typing.Optional[str]: total_weight = 0 fractions = [] for arg in args: - fraction = __parse_fraction(arg) + fraction = _parse_fraction(arg) if fraction: fractions.append(fraction) total_weight += fraction.weight @@ -58,7 +58,7 @@ def fractional(data: dict, *args: JsonLogicArg) -> typing.Optional[str]: return None -def __parse_fraction(arg: JsonLogicArg) -> typing.Optional[Fraction]: +def _parse_fraction(arg: JsonLogicArg) -> typing.Optional[Fraction]: if not isinstance(arg, (tuple, list)) or not arg: logger.error( "Fractional variant weights must be (str, int) tuple or [str] list"