From 72d1ee5cd946559a38c954fcbf9f2dd26e2a972d Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Mon, 4 Mar 2024 17:35:28 -0800 Subject: [PATCH 01/40] wip --- eppo_client/__init__.py | 4 +- eppo_client/client.py | 111 ++------ eppo_client/config.py | 2 +- eppo_client/configuration_requestor.py | 34 +-- eppo_client/eval.py | 61 +++++ eppo_client/http_client.py | 2 +- eppo_client/models.py | 45 +++ eppo_client/rules.py | 26 +- eppo_client/shard.py | 20 -- eppo_client/sharding.py | 30 ++ eppo_client/variation_type.py | 4 +- test/eval_test.py | 234 ++++++++++++++++ test/rules_test.py | 363 ++++++++++++++++--------- 13 files changed, 646 insertions(+), 290 deletions(-) create mode 100644 eppo_client/eval.py create mode 100644 eppo_client/models.py delete mode 100644 eppo_client/shard.py create mode 100644 eppo_client/sharding.py create mode 100644 test/eval_test.py diff --git a/eppo_client/__init__.py b/eppo_client/__init__.py index 67196d9..6fbe3f5 100644 --- a/eppo_client/__init__.py +++ b/eppo_client/__init__.py @@ -2,12 +2,12 @@ from eppo_client.client import EppoClient from eppo_client.config import Config from eppo_client.configuration_requestor import ( - ExperimentConfigurationDto, ExperimentConfigurationRequestor, ) from eppo_client.configuration_store import ConfigurationStore from eppo_client.constants import MAX_CACHE_ENTRIES from eppo_client.http_client import HttpClient, SdkParams +from eppo_client.models import Flag from eppo_client.read_write_lock import ReadWriteLock __version__ = "1.3.1" @@ -31,7 +31,7 @@ def init(config: Config) -> EppoClient: apiKey=config.api_key, sdkName="python", sdkVersion=__version__ ) http_client = HttpClient(base_url=config.base_url, sdk_params=sdk_params) - config_store: ConfigurationStore[ExperimentConfigurationDto] = ConfigurationStore( + config_store: ConfigurationStore[Flag] = ConfigurationStore( max_size=MAX_CACHE_ENTRIES ) config_requestor = ExperimentConfigurationRequestor( diff --git a/eppo_client/client.py b/eppo_client/client.py index 1db6ddf..5c941e7 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -6,16 +6,15 @@ from numbers import Number from eppo_client.assignment_logger import AssignmentLogger from eppo_client.configuration_requestor import ( - ExperimentConfigurationDto, ExperimentConfigurationRequestor, - VariationDto, ) from eppo_client.constants import POLL_INTERVAL_MILLIS, POLL_JITTER_MILLIS from eppo_client.poller import Poller -from eppo_client.rules import find_matching_rule -from eppo_client.shard import ShardRange, get_shard, is_in_shard_range +from eppo_client.sharding import MD5Sharder from eppo_client.validation import validate_not_blank from eppo_client.variation_type import VariationType +from eppo_client.eval import Evaluator +from eppo_client.models import Variation, Flag logger = logging.getLogger(__name__) @@ -36,6 +35,7 @@ def __init__( callback=config_requestor.fetch_and_store_configurations, ) self.__poller.start() + self.__evaluator = Evaluator(sharder=MD5Sharder()) def get_string_assignment( self, subject_key: str, flag_key: str, subject_attributes=dict() @@ -172,7 +172,7 @@ def get_assignment_variation( flag_key: str, subject_attributes: Any, expected_variation_type: Optional[str] = None, - ) -> Optional[VariationDto]: + ) -> Optional[Variation]: """Maps a subject to a variation for a given experiment Returns None if the subject is not part of the experiment sample. @@ -183,114 +183,35 @@ def get_assignment_variation( """ validate_not_blank("subject_key", subject_key) validate_not_blank("flag_key", flag_key) - experiment_config = self.__config_requestor.get_configuration(flag_key) - override = self._get_subject_variation_override(experiment_config, subject_key) - if override: - if expected_variation_type is not None: - variation_is_expected_type = VariationType.is_expected_type( - override, expected_variation_type - ) - if not variation_is_expected_type: - return None - return override + flag = self.__config_requestor.get_configuration(flag_key) - if experiment_config is None or not experiment_config.enabled: + if flag is None or not flag.enabled: logger.info( - "[Eppo SDK] No assigned variation. No active experiment or flag for key: " - + flag_key + "[Eppo SDK] No assigned variation. No active flag for key: " + flag_key ) return None - matched_rule = find_matching_rule(subject_attributes, experiment_config.rules) - if matched_rule is None: - logger.info( - "[Eppo SDK] No assigned variation. Subject attributes do not match targeting rules: {0}".format( - subject_attributes - ) - ) - return None - - allocation = experiment_config.allocations[matched_rule.allocation_key] - if not self._is_in_experiment_sample( - subject_key, - flag_key, - experiment_config.subject_shards, - allocation.percent_exposure, - ): - logger.info( - "[Eppo SDK] No assigned variation. Subject is not part of experiment sample population" - ) - return None - - shard = get_shard( - "assignment-{}-{}".format(subject_key, flag_key), - experiment_config.subject_shards, - ) - assigned_variation = next( - ( - variation - for variation in allocation.variations - if is_in_shard_range(shard, variation.shard_range) - ), - None, - ) - - assigned_variation_value_to_log = None - if assigned_variation is not None: - assigned_variation_value_to_log = assigned_variation.value - if expected_variation_type is not None: - variation_is_expected_type = VariationType.is_expected_type( - assigned_variation, expected_variation_type - ) - if not variation_is_expected_type: - return None + result = self.__evaluator.evaluate_flag(flag, subject_key, subject_attributes) assignment_event = { - "allocation": matched_rule.allocation_key, - "experiment": f"{flag_key}-{matched_rule.allocation_key}", + **result.extra_logging, + "allocation": result.allocation_key, + "experiment": f"{flag_key}-{result.allocation_key}", "featureFlag": flag_key, - "variation": assigned_variation_value_to_log, + "variation": result.variation.key, "subject": subject_key, "timestamp": datetime.datetime.utcnow().isoformat(), "subjectAttributes": subject_attributes, } try: - self.__assignment_logger.log_assignment(assignment_event) + if result.do_log: + self.__assignment_logger.log_assignment(assignment_event) except Exception as e: logger.error("[Eppo SDK] Error logging assignment event: " + str(e)) - return assigned_variation + return result.variation def _shutdown(self): """Stops all background processes used by the client Do not use the client after calling this method. """ self.__poller.stop() - - def _get_subject_variation_override( - self, experiment_config: Optional[ExperimentConfigurationDto], subject: str - ) -> Optional[VariationDto]: - subject_hash = hashlib.md5(subject.encode("utf-8")).hexdigest() - if ( - experiment_config is not None - and subject_hash in experiment_config.overrides - ): - return VariationDto( - name="override", - value=experiment_config.overrides[subject_hash], - typed_value=experiment_config.typed_overrides[subject_hash], - shard_range=ShardRange(start=0, end=10000), - ) - return None - - def _is_in_experiment_sample( - self, - subject: str, - experiment_key: str, - subject_shards: int, - percent_exposure: float, - ): - shard = get_shard( - "exposure-{}-{}".format(subject, experiment_key), - subject_shards, - ) - return shard <= percent_exposure * subject_shards diff --git a/eppo_client/config.py b/eppo_client/config.py index d9d7d88..17a4106 100644 --- a/eppo_client/config.py +++ b/eppo_client/config.py @@ -1,5 +1,5 @@ from eppo_client.assignment_logger import AssignmentLogger -from eppo_client.base_model import SdkBaseModel +from eppo_client.models import SdkBaseModel from eppo_client.validation import validate_not_blank diff --git a/eppo_client/configuration_requestor.py b/eppo_client/configuration_requestor.py index a8fb2ee..cefc568 100644 --- a/eppo_client/configuration_requestor.py +++ b/eppo_client/configuration_requestor.py @@ -4,33 +4,11 @@ from eppo_client.configuration_store import ConfigurationStore from eppo_client.http_client import HttpClient from eppo_client.rules import Rule -from eppo_client.shard import ShardRange +from eppo_client.models import Flag logger = logging.getLogger(__name__) -class VariationDto(SdkBaseModel): - name: str - value: str - typed_value: Any = None - shard_range: ShardRange - - -class AllocationDto(SdkBaseModel): - percent_exposure: float - variations: List[VariationDto] - - -class ExperimentConfigurationDto(SdkBaseModel): - subject_shards: int - enabled: bool - name: Optional[str] = None - overrides: Dict[str, str] = {} - typed_overrides: Dict[str, Any] = {} - rules: List[Rule] = [] - allocations: Dict[str, AllocationDto] - - RAC_ENDPOINT = "/randomized_assignment/v3/config" @@ -38,23 +16,21 @@ class ExperimentConfigurationRequestor: def __init__( self, http_client: HttpClient, - config_store: ConfigurationStore[ExperimentConfigurationDto], + config_store: ConfigurationStore[Flag], ): self.__http_client = http_client self.__config_store = config_store - def get_configuration( - self, experiment_key: str - ) -> Optional[ExperimentConfigurationDto]: + def get_configuration(self, experiment_key: str) -> Optional[Flag]: if self.__http_client.is_unauthorized(): raise ValueError("Unauthorized: please check your API key") return self.__config_store.get_configuration(experiment_key) - def fetch_and_store_configurations(self) -> Dict[str, ExperimentConfigurationDto]: + def fetch_and_store_configurations(self) -> Dict[str, Flag]: try: configs = cast(dict, self.__http_client.get(RAC_ENDPOINT).get("flags", {})) for exp_key, exp_config in configs.items(): - configs[exp_key] = ExperimentConfigurationDto(**exp_config) + configs[exp_key] = Flag(**exp_config) self.__config_store.set_configurations(configs) return configs except Exception as e: diff --git a/eppo_client/eval.py b/eppo_client/eval.py new file mode 100644 index 0000000..36a01e3 --- /dev/null +++ b/eppo_client/eval.py @@ -0,0 +1,61 @@ +from typing import Dict, Optional +from eppo_client.sharding import Sharder +from eppo_client.models import Range, Shard, Variation +from eppo_client.rules import matches_rule +import hashlib +from dataclasses import dataclass +import logging + + +@dataclass +class EvalResult: + flag_key: str + allocation_key: str + variation: Variation + extra_logging: Dict[str, str] + do_log: bool + + +@dataclass +class Evaluator: + sharder: Sharder + + def evaluate_flag( + self, flag, subject_key, subject_attributes + ) -> Optional[EvalResult]: + for allocation in flag.allocations: + if not allocation.rules or any( + matches_rule(rule, subject_attributes) for rule in allocation.rules + ): + for split in allocation.splits: + if all( + self.matches_shard(shard, subject_key, flag.total_shards) + for shard in split.shards + ): + return EvalResult( + flag_key=flag.key, + allocation_key=allocation.key, + variation=flag.variations.get(split.variation_key), + extra_logging=split.extra_logging, + do_log=allocation.do_log, + ) + + return EvalResult( + flag_key=flag.key, + allocation_key=None, + variation=None, + extra_logging={}, + do_log=True, + ) + + def matches_shard(self, shard: Shard, subject_key: str, total_shards: int) -> bool: + h = self.sharder.get_shard(seed(shard.salt, subject_key), total_shards) + return any(is_in_shard_range(h, r) for r in shard.ranges) + + +def is_in_shard_range(shard: int, range: Range) -> bool: + return range.start <= shard < range.end + + +def seed(salt, subject_key): + return f"{salt}-{subject_key}" diff --git a/eppo_client/http_client.py b/eppo_client/http_client.py index 4b6ea52..388b074 100644 --- a/eppo_client/http_client.py +++ b/eppo_client/http_client.py @@ -5,7 +5,7 @@ import requests -from eppo_client.base_model import SdkBaseModel +from eppo_client.models import SdkBaseModel class SdkParams(SdkBaseModel): diff --git a/eppo_client/models.py b/eppo_client/models.py new file mode 100644 index 0000000..bb8850a --- /dev/null +++ b/eppo_client/models.py @@ -0,0 +1,45 @@ +from datetime import datetime + +from typing import Dict, List, Optional + +from eppo_client.base_model import SdkBaseModel +from eppo_client.rules import Rule + + +class Variation(SdkBaseModel): + # TODO: generalize + key: str + value: str + + +class Range(SdkBaseModel): + start: int + end: int + + +class Shard(SdkBaseModel): + salt: str + ranges: List[Range] + + +class Split(SdkBaseModel): + shards: List[Shard] + variation_key: str + extra_logging: Dict[str, str] = {} + + +class Allocation(SdkBaseModel): + key: str + rules: List[Rule] + start_at: Optional[datetime] = None + end_at: Optional[datetime] = None + splits: List[Split] + do_log: bool = True + + +class Flag(SdkBaseModel): + key: str + enabled: bool + variations: Dict[str, Variation] + allocations: List[Allocation] + total_shards: int = 10_000 diff --git a/eppo_client/rules.py b/eppo_client/rules.py index 84b1055..4a811a4 100644 --- a/eppo_client/rules.py +++ b/eppo_client/rules.py @@ -2,9 +2,9 @@ import re import semver from enum import Enum -from typing import Any, List +from typing import Any, List, Optional, Dict -from eppo_client.base_model import SdkBaseModel +from eppo_client.models import SdkBaseModel class OperatorType(Enum): @@ -24,25 +24,19 @@ class Condition(SdkBaseModel): class Rule(SdkBaseModel): - allocation_key: str conditions: List[Condition] -def find_matching_rule(subject_attributes: dict, rules: List[Rule]): - for rule in rules: - if matches_rule(subject_attributes, rule): - return rule - return None +def matches_rule(rule: Rule, subject_attributes: Dict[str, str]) -> bool: + return all( + evaluate_condition(condition, subject_attributes) + for condition in rule.conditions + ) -def matches_rule(subject_attributes: dict, rule: Rule): - for condition in rule.conditions: - if not evaluate_condition(subject_attributes, condition): - return False - return True - - -def evaluate_condition(subject_attributes: dict, condition: Condition) -> bool: +def evaluate_condition( + condition: Condition, subject_attributes: Dict[str, str] +) -> bool: subject_value = subject_attributes.get(condition.attribute, None) if subject_value is not None: if condition.operator == OperatorType.MATCHES: diff --git a/eppo_client/shard.py b/eppo_client/shard.py deleted file mode 100644 index 5183ba5..0000000 --- a/eppo_client/shard.py +++ /dev/null @@ -1,20 +0,0 @@ -import hashlib - -from eppo_client.base_model import SdkBaseModel - - -def get_shard(input: str, subject_shards: int): - hash_output = hashlib.md5(input.encode("utf-8")).hexdigest() - # get the first 4 bytes of the md5 hex string and parse it using base 16 - # (8 hex characters represent 4 bytes, e.g. 0xffffffff represents the max 4-byte integer) - int_from_hash = int(hash_output[0:8], 16) - return int_from_hash % subject_shards - - -class ShardRange(SdkBaseModel): - start: int - end: int - - -def is_in_shard_range(shard: int, range: ShardRange) -> bool: - return shard >= range.start and shard < range.end diff --git a/eppo_client/sharding.py b/eppo_client/sharding.py new file mode 100644 index 0000000..7b9fb36 --- /dev/null +++ b/eppo_client/sharding.py @@ -0,0 +1,30 @@ +from abc import ABC, abstractmethod +from typing import Dict +import hashlib + + +class Sharder(ABC): + @abstractmethod + def get_shard(self, input: str, total_shards: int) -> int: ... + + +class MD5Sharder(Sharder): + def get_shard(self, input: str, total_shards: int) -> int: + hash_output = hashlib.md5(input.encode("utf-8")).hexdigest() + # get the first 4 bytes of the md5 hex string and parse it using base 16 + # (8 hex characters represent 4 bytes, e.g. 0xffffffff represents the max 4-byte integer) + int_from_hash = int(hash_output[0:8], 16) + return int_from_hash % total_shards + + +class DeterministicSharder(Sharder): + """ + Deterministic sharding based on a look-up table + to simplify writing tests + """ + + def __init__(self, lookup: Dict[str, str]): + self.lookup = lookup + + def get_shard(self, input: str, total_shards: int) -> int: + return self.lookup.get(input, 0) diff --git a/eppo_client/variation_type.py b/eppo_client/variation_type.py index 0b7a013..29725a7 100644 --- a/eppo_client/variation_type.py +++ b/eppo_client/variation_type.py @@ -1,6 +1,6 @@ import json from numbers import Number -from eppo_client.configuration_requestor import VariationDto +from eppo_client.models import Variation class VariationType: @@ -11,7 +11,7 @@ class VariationType: @classmethod def is_expected_type( - cls, assigned_variation: VariationDto, expected_variation_type: str + cls, assigned_variation: Variation, expected_variation_type: str ) -> bool: if expected_variation_type == cls.STRING: return isinstance(assigned_variation.typed_value, str) diff --git a/test/eval_test.py b/test/eval_test.py new file mode 100644 index 0000000..5a087b8 --- /dev/null +++ b/test/eval_test.py @@ -0,0 +1,234 @@ +from eppo_client.models import Flag, Allocation, Range, Variation, Split, Shard +from eppo_client.eval import Evaluator, EvalResult +from eppo_client.rules import Condition, OperatorType, Rule +from eppo_client.sharding import DeterministicSharder, MD5Sharder + + +def test_matches_shard_full_range(): + shard = Shard( + salt="a", + ranges=[Range(start=0, end=100)], + ) + + evaluator = Evaluator(sharder=MD5Sharder()) + assert evaluator.matches_shard(shard, "subject_key", 100) is True + + +def test_matches_shard_full_range_split(): + shard = Shard( + salt="a", + ranges=[Range(start=0, end=50), Range(start=50, end=100)], + ) + + evaluator = Evaluator(sharder=MD5Sharder()) + assert evaluator.matches_shard(shard, "subject_key", 100) is True + + +def test_matches_shard_no_match(): + shard = Shard( + salt="a", + ranges=[Range(start=0, end=50)], + ) + + evaluator = Evaluator(sharder=DeterministicSharder({"a-subject_key": 99})) + assert evaluator.matches_shard(shard, "subject_key", 100) is False + + +def test_eval_empty_flag(): + empty_flag = Flag( + key="empty", + enabled=True, + variations={ + "a": Variation(key="a", value="A"), + "b": Variation(key="b", value="B"), + }, + allocations=[], + total_shards=10, + ) + + evaluator = Evaluator(sharder=MD5Sharder()) + assert evaluator.evaluate_flag(empty_flag, "subject_key", {}) == EvalResult( + flag_key="empty", + allocation_key=None, + variation=None, + extra_logging={}, + do_log=True, + ) + + +def test_catch_all_allocation(): + flag = Flag( + key="flag", + enabled=True, + variations={ + "a": Variation(key="a", value="A"), + "b": Variation(key="b", value="B"), + }, + allocations=[ + Allocation( + key="default", + rules=[], + splits=[Split(variation_key="a", shards=[])], + ) + ], + total_shards=10, + ) + + evaluator = Evaluator(sharder=MD5Sharder()) + result = evaluator.evaluate_flag(flag, "subject_key", {}) + assert result.flag_key == "flag" + assert result.allocation_key == "default" + assert result.variation == Variation(key="a", value="A") + + +def test_match_first_allocation_rule(): + flag = Flag( + key="flag", + enabled=True, + variations={ + "a": Variation(key="a", value="A"), + "b": Variation(key="b", value="B"), + }, + allocations=[ + Allocation( + key="first", + rules=[ + Rule( + conditions=[ + Condition( + operator=OperatorType.MATCHES, + attribute="email", + value=".*@example.com", + ) + ] + ) + ], + splits=[Split(variation_key="b", shards=[])], + ), + Allocation( + key="default", + rules=[], + splits=[Split(variation_key="a", shards=[])], + ), + ], + total_shards=10, + ) + + evaluator = Evaluator(sharder=MD5Sharder()) + result = evaluator.evaluate_flag(flag, "subject_key", {"email": "eppo@example.com"}) + assert result.flag_key == "flag" + assert result.allocation_key == "first" + assert result.variation == Variation(key="b", value="B") + + +def test_do_not_match_first_allocation_rule(): + flag = Flag( + key="flag", + enabled=True, + variations={ + "a": Variation(key="a", value="A"), + "b": Variation(key="b", value="B"), + }, + allocations=[ + Allocation( + key="first", + rules=[ + Rule( + conditions=[ + Condition( + operator=OperatorType.MATCHES, + attribute="email", + value=".*@example.com", + ) + ] + ) + ], + splits=[Split(variation_key="b", shards=[])], + ), + Allocation( + key="default", + rules=[], + splits=[Split(variation_key="a", shards=[])], + ), + ], + total_shards=10, + ) + + evaluator = Evaluator(sharder=MD5Sharder()) + result = evaluator.evaluate_flag(flag, "subject_key", {"email": "eppo@test.com"}) + assert result.flag_key == "flag" + assert result.allocation_key == "default" + assert result.variation == Variation(key="a", value="A") + + +def test_eval_sharding(): + flag = Flag( + key="flag", + enabled=True, + variations={ + "a": Variation(key="a", value="A"), + "b": Variation(key="b", value="B"), + "c": Variation(key="c", value="C"), + }, + allocations=[ + Allocation( + key="first", + rules=[], + splits=[ + Split( + variation_key="a", + shards=[ + Shard(salt="traffic", ranges=[Range(start=0, end=5)]), + Shard(salt="split", ranges=[Range(start=0, end=3)]), + ], + ), + Split( + variation_key="b", + shards=[ + Shard(salt="traffic", ranges=[Range(start=0, end=5)]), + Shard(salt="split", ranges=[Range(start=3, end=6)]), + ], + ), + ], + ), + Allocation( + key="default", + rules=[], + splits=[Split(variation_key="c", shards=[])], + ), + ], + total_shards=10, + ) + + evaluator = Evaluator( + sharder=DeterministicSharder( + { + "traffic-alice": 2, + "traffic-bob": 3, + "traffic-charlie": 4, + "traffic-dave": 7, + "split-alice": 1, + "split-bob": 4, + "split-charlie": 8, + "split-dave": 1, + } + ) + ) + + result = evaluator.evaluate_flag(flag, "alice", {}) + assert result.allocation_key == "first" + assert result.variation == Variation(key="a", value="A") + + result = evaluator.evaluate_flag(flag, "bob", {}) + assert result.allocation_key == "first" + assert result.variation == Variation(key="b", value="B") + + # charlie matches on traffic but not on split and falls through + result = evaluator.evaluate_flag(flag, "charlie", {}) + assert result.allocation_key == "default" + assert result.variation == Variation(key="c", value="C") + + # dave does not match traffic + result = evaluator.evaluate_flag(flag, "dave", {}) + assert result.allocation_key == "default" + assert result.variation == Variation(key="c", value="C") diff --git a/test/rules_test.py b/test/rules_test.py index 65089d2..2155526 100644 --- a/test/rules_test.py +++ b/test/rules_test.py @@ -2,7 +2,8 @@ OperatorType, Rule, Condition, - find_matching_rule, + evaluate_condition, + matches_rule, ) greater_than_condition = Condition(operator=OperatorType.GT, value=10, attribute="age") @@ -20,151 +21,265 @@ rule_with_empty_conditions = Rule(allocation_key="allocation", conditions=[]) -def test_find_matching_rule_with_empty_rules(): - subject_attributes = {"age": 20, "country": "US"} - assert find_matching_rule(subject_attributes, []) is None +def test_matches_rule_with_empty_rule(): + assert matches_rule(Rule(conditions=[]), {}) -def test_find_matching_rule_when_no_rules_match(): - subject_attributes = {"age": 99, "country": "US", "email": "test@example.com"} - assert find_matching_rule(subject_attributes, [text_rule]) is None +def test_matches_rule_with_single_condition(): + assert matches_rule( + Rule( + conditions=[Condition(operator=OperatorType.GT, value=10, attribute="age")] + ), + {"age": 11}, + ) -def test_find_matching_rule_on_match(): - assert find_matching_rule({"age": 99}, [numeric_rule]) == numeric_rule - assert find_matching_rule({"email": "testing@email.com"}, [text_rule]) == text_rule +def test_matches_rule_with_single_condition_missing_attribute(): + assert not matches_rule( + Rule( + conditions=[Condition(operator=OperatorType.GT, value=10, attribute="age")] + ), + {"name": "alice"}, + ) -def test_find_matching_rule_if_no_attribute_for_condition(): - assert find_matching_rule({}, [numeric_rule]) is None +def test_matches_rule_with_single_false_condition(): + assert not matches_rule( + Rule( + conditions=[Condition(operator=OperatorType.GT, value=10, attribute="age")] + ), + {"age": 9}, + ) -def test_find_matching_rule_if_no_conditions_for_rule(): - assert ( - find_matching_rule({}, [rule_with_empty_conditions]) - == rule_with_empty_conditions +def test_matches_rule_with_two_conditions(): + assert matches_rule( + Rule( + conditions=[ + Condition(operator=OperatorType.GT, value=10, attribute="age"), + Condition(operator=OperatorType.LT, value=100, attribute="age"), + ] + ), + {"age": 20}, ) -def test_find_matching_rule_if_numeric_operator_with_string(): - assert find_matching_rule({"age": "99"}, [numeric_rule]) is None +def test_matches_rule_with_true_and_false_condition(): + assert not matches_rule( + Rule( + conditions=[ + Condition(operator=OperatorType.GT, value=10, attribute="age"), + Condition(operator=OperatorType.LT, value=20, attribute="age"), + ] + ), + {"age": 30}, + ) -def test_find_matching_rule_with_numeric_value_and_regex(): - condition = Condition( - operator=OperatorType.MATCHES, value="[0-9]+", attribute="age" +def test_evaluate_condition_matches(): + assert evaluate_condition( + Condition(operator=OperatorType.MATCHES, value=".*", attribute="name"), + {"name": "alice"}, ) - rule = Rule(conditions=[condition], allocation_key="allocation") - assert find_matching_rule({"age": 99}, [rule]) == rule -def test_find_matching_rule_with_semver(): - semver_greater_than_condition = Condition( - operator=OperatorType.GTE, value="1.0.0", attribute="version" - ) - semver_less_than_condition = Condition( - operator=OperatorType.LTE, value="2.0.0", attribute="version" +def test_evaluate_condition_matches_false(): + assert not evaluate_condition( + Condition(operator=OperatorType.MATCHES, value="bob", attribute="name"), + {"name": "alice"}, ) - semver_rule = Rule( - allocation_key="allocation", - conditions=[semver_less_than_condition, semver_greater_than_condition], - ) - - assert find_matching_rule({"version": "1.1.0"}, [semver_rule]) is semver_rule - assert find_matching_rule({"version": "2.0.0"}, [semver_rule]) is semver_rule - assert find_matching_rule({"version": "2.1.0"}, [semver_rule]) is None -def test_one_of_operator_with_boolean(): - oneOfRule = Rule( - allocation_key="allocation", - conditions=[ - Condition(operator=OperatorType.ONE_OF, value=["True"], attribute="enabled") - ], - ) - notOneOfRule = Rule( - allocation_key="allocation", - conditions=[ - Condition( - operator=OperatorType.NOT_ONE_OF, value=["True"], attribute="enabled" - ) - ], +def test_evaluate_condition_one_of(): + assert evaluate_condition( + Condition( + operator=OperatorType.ONE_OF, value=["alice", "bob"], attribute="name" + ), + {"name": "alice"}, ) - assert find_matching_rule({"enabled": True}, [oneOfRule]) == oneOfRule - assert find_matching_rule({"enabled": False}, [oneOfRule]) is None - assert find_matching_rule({"enabled": True}, [notOneOfRule]) is None - assert find_matching_rule({"enabled": False}, [notOneOfRule]) == notOneOfRule - - -def test_one_of_operator_case_insensitive(): - oneOfRule = Rule( - allocation_key="allocation", - conditions=[ - Condition( - operator=OperatorType.ONE_OF, value=["1Ab", "Ron"], attribute="name" - ) - ], + assert evaluate_condition( + Condition( + operator=OperatorType.ONE_OF, value=["alice", "bob"], attribute="name" + ), + {"name": "bob"}, ) - assert find_matching_rule({"name": "ron"}, [oneOfRule]) == oneOfRule - assert find_matching_rule({"name": "1AB"}, [oneOfRule]) == oneOfRule - - -def test_not_one_of_operator_case_insensitive(): - notOneOf = Rule( - allocation_key="allocation", - conditions=[ - Condition( - operator=OperatorType.NOT_ONE_OF, - value=["bbB", "1.1.ab"], - attribute="name", - ) - ], + assert not evaluate_condition( + Condition( + operator=OperatorType.ONE_OF, value=["alice", "bob"], attribute="name" + ), + {"name": "charlie"}, ) - assert find_matching_rule({"name": "BBB"}, [notOneOf]) is None - assert find_matching_rule({"name": "1.1.AB"}, [notOneOf]) is None - - -def test_one_of_operator_with_string(): - oneOfRule = Rule( - allocation_key="allocation", - conditions=[ - Condition( - operator=OperatorType.ONE_OF, value=["john", "ron"], attribute="name" - ) - ], - ) - notOneOfRule = Rule( - allocation_key="allocation", - conditions=[ - Condition(operator=OperatorType.NOT_ONE_OF, value=["ron"], attribute="name") - ], + + +def test_evaluate_condition_not_one_of(): + assert not evaluate_condition( + Condition( + operator=OperatorType.NOT_ONE_OF, value=["alice", "bob"], attribute="name" + ), + {"name": "alice"}, ) - assert find_matching_rule({"name": "john"}, [oneOfRule]) == oneOfRule - assert find_matching_rule({"name": "ron"}, [oneOfRule]) == oneOfRule - assert find_matching_rule({"name": "sam"}, [oneOfRule]) is None - assert find_matching_rule({"name": "ron"}, [notOneOfRule]) is None - assert find_matching_rule({"name": "sam"}, [notOneOfRule]) == notOneOfRule - - -def test_one_of_operator_with_number(): - oneOfRule = Rule( - allocation_key="allocation", - conditions=[ - Condition( - operator=OperatorType.ONE_OF, value=["14", "15.11"], attribute="number" - ) - ], + assert not evaluate_condition( + Condition( + operator=OperatorType.NOT_ONE_OF, value=["alice", "bob"], attribute="name" + ), + {"name": "bob"}, ) - notOneOfRule = Rule( - allocation_key="allocation", - conditions=[ - Condition( - operator=OperatorType.NOT_ONE_OF, value=["10"], attribute="number" - ) - ], + assert evaluate_condition( + Condition( + operator=OperatorType.NOT_ONE_OF, value=["alice", "bob"], attribute="name" + ), + {"name": "charlie"}, ) - assert find_matching_rule({"number": "14"}, [oneOfRule]) == oneOfRule - assert find_matching_rule({"number": 15.11}, [oneOfRule]) == oneOfRule - assert find_matching_rule({"number": "10"}, [oneOfRule]) is None - assert find_matching_rule({"number": "10"}, [notOneOfRule]) is None - assert find_matching_rule({"number": 11}, [notOneOfRule]) == notOneOfRule + + +# def test_find_matching_rule_with_empty_rules(): +# subject_attributes = {"age": 20, "country": "US"} +# assert find_matching_rule([], "alice", subject_attributes) is None + + +# def test_find_matching_rule_when_no_rules_match(): +# subject_attributes = {"age": 99, "country": "US", "email": "test@example.com"} +# assert find_matching_rule([text_rule], "alice", subject_attributes) is None + + +# def test_find_matching_rule_on_match(): +# assert find_matching_rule([numeric_rule], "alice", {"age": 99}) == numeric_rule +# assert ( +# find_matching_rule([text_rule], "alice", {"email": "testing@email.com"}) +# == text_rule +# ) + + +# def test_find_matching_rule_if_no_attribute_for_condition(): +# assert find_matching_rule([numeric_rule], "alice", {}) is None + + +# def test_find_matching_rule_if_no_conditions_for_rule(): +# assert ( +# find_matching_rule([rule_with_empty_conditions], "alice", {}) +# == rule_with_empty_conditions +# ) + + +# def test_find_matching_rule_if_numeric_operator_with_string(): +# assert find_matching_rule([numeric_rule], "alice", {"age": "99"}) is None + + +# def test_find_matching_rule_with_numeric_value_and_regex(): +# condition = Condition( +# operator=OperatorType.MATCHES, value="[0-9]+", attribute="age" +# ) +# rule = Rule(conditions=[condition], allocation_key="allocation") +# assert find_matching_rule([rule], "alice", {"age": 99}) == rule + + +# def test_find_matching_rule_with_semver(): +# semver_greater_than_condition = Condition( +# operator=OperatorType.GTE, value="1.0.0", attribute="version" +# ) +# semver_less_than_condition = Condition( +# operator=OperatorType.LTE, value="2.0.0", attribute="version" +# ) +# semver_rule = Rule( +# allocation_key="allocation", +# conditions=[semver_less_than_condition, semver_greater_than_condition], +# ) + +# assert find_matching_rule({"version": "1.1.0"}, [semver_rule]) is semver_rule +# assert find_matching_rule({"version": "2.0.0"}, [semver_rule]) is semver_rule +# assert find_matching_rule({"version": "2.1.0"}, [semver_rule]) is None + + +# def test_one_of_operator_with_boolean(): +# oneOfRule = Rule( +# allocation_key="allocation", +# conditions=[ +# Condition(operator=OperatorType.ONE_OF, value=["True"], attribute="enabled") +# ], +# ) +# notOneOfRule = Rule( +# allocation_key="allocation", +# conditions=[ +# Condition( +# operator=OperatorType.NOT_ONE_OF, value=["True"], attribute="enabled" +# ) +# ], +# ) +# assert find_matching_rule({"enabled": True}, [oneOfRule]) == oneOfRule +# assert find_matching_rule({"enabled": False}, [oneOfRule]) is None +# assert find_matching_rule({"enabled": True}, [notOneOfRule]) is None +# assert find_matching_rule({"enabled": False}, [notOneOfRule]) == notOneOfRule + + +# def test_one_of_operator_case_insensitive(): +# oneOfRule = Rule( +# allocation_key="allocation", +# conditions=[ +# Condition( +# operator=OperatorType.ONE_OF, value=["1Ab", "Ron"], attribute="name" +# ) +# ], +# ) +# assert find_matching_rule({"name": "ron"}, [oneOfRule]) == oneOfRule +# assert find_matching_rule({"name": "1AB"}, [oneOfRule]) == oneOfRule + + +# def test_not_one_of_operator_case_insensitive(): +# notOneOf = Rule( +# allocation_key="allocation", +# conditions=[ +# Condition( +# operator=OperatorType.NOT_ONE_OF, +# value=["bbB", "1.1.ab"], +# attribute="name", +# ) +# ], +# ) +# assert find_matching_rule({"name": "BBB"}, [notOneOf]) is None +# assert find_matching_rule({"name": "1.1.AB"}, [notOneOf]) is None + + +# def test_one_of_operator_with_string(): +# oneOfRule = Rule( +# allocation_key="allocation", +# conditions=[ +# Condition( +# operator=OperatorType.ONE_OF, value=["john", "ron"], attribute="name" +# ) +# ], +# ) +# notOneOfRule = Rule( +# allocation_key="allocation", +# conditions=[ +# Condition(operator=OperatorType.NOT_ONE_OF, value=["ron"], attribute="name") +# ], +# ) +# assert find_matching_rule({"name": "john"}, [oneOfRule]) == oneOfRule +# assert find_matching_rule({"name": "ron"}, [oneOfRule]) == oneOfRule +# assert find_matching_rule({"name": "sam"}, [oneOfRule]) is None +# assert find_matching_rule({"name": "ron"}, [notOneOfRule]) is None +# assert find_matching_rule({"name": "sam"}, [notOneOfRule]) == notOneOfRule + + +# def test_one_of_operator_with_number(): +# oneOfRule = Rule( +# allocation_key="allocation", +# conditions=[ +# Condition( +# operator=OperatorType.ONE_OF, value=["14", "15.11"], attribute="number" +# ) +# ], +# ) +# notOneOfRule = Rule( +# allocation_key="allocation", +# conditions=[ +# Condition( +# operator=OperatorType.NOT_ONE_OF, value=["10"], attribute="number" +# ) +# ], +# ) +# assert find_matching_rule({"number": "14"}, [oneOfRule]) == oneOfRule +# assert find_matching_rule({"number": 15.11}, [oneOfRule]) == oneOfRule +# assert find_matching_rule({"number": "10"}, [oneOfRule]) is None +# assert find_matching_rule({"number": "10"}, [notOneOfRule]) is None +# assert find_matching_rule({"number": 11}, [notOneOfRule]) == notOneOfRule From 35b6151e095ad03f8ed3009ebe1fd602f41aa499 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Mon, 4 Mar 2024 17:40:32 -0800 Subject: [PATCH 02/40] add todo on timestamps --- eppo_client/eval.py | 1 + 1 file changed, 1 insertion(+) diff --git a/eppo_client/eval.py b/eppo_client/eval.py index 36a01e3..f1d80be 100644 --- a/eppo_client/eval.py +++ b/eppo_client/eval.py @@ -24,6 +24,7 @@ def evaluate_flag( self, flag, subject_key, subject_attributes ) -> Optional[EvalResult]: for allocation in flag.allocations: + # todo: add conditionals for allocation start and end timestamps if not allocation.rules or any( matches_rule(rule, subject_attributes) for rule in allocation.rules ): From a3a48332ecc28c1754c393c0b3ead34e1b15b42e Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Mon, 4 Mar 2024 21:41:45 -0800 Subject: [PATCH 03/40] fix linting errors --- eppo_client/client.py | 3 +-- eppo_client/configuration_requestor.py | 4 +--- eppo_client/eval.py | 2 -- eppo_client/rules.py | 2 +- 4 files changed, 3 insertions(+), 8 deletions(-) diff --git a/eppo_client/client.py b/eppo_client/client.py index 5c941e7..71dca69 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -1,4 +1,3 @@ -import hashlib import datetime import logging from typing import Any, Dict, Optional @@ -14,7 +13,7 @@ from eppo_client.validation import validate_not_blank from eppo_client.variation_type import VariationType from eppo_client.eval import Evaluator -from eppo_client.models import Variation, Flag +from eppo_client.models import Variation logger = logging.getLogger(__name__) diff --git a/eppo_client/configuration_requestor.py b/eppo_client/configuration_requestor.py index cefc568..6592e39 100644 --- a/eppo_client/configuration_requestor.py +++ b/eppo_client/configuration_requestor.py @@ -1,9 +1,7 @@ import logging -from typing import Any, Dict, List, Optional, cast -from eppo_client.base_model import SdkBaseModel +from typing import Dict, Optional, cast from eppo_client.configuration_store import ConfigurationStore from eppo_client.http_client import HttpClient -from eppo_client.rules import Rule from eppo_client.models import Flag logger = logging.getLogger(__name__) diff --git a/eppo_client/eval.py b/eppo_client/eval.py index f1d80be..353d057 100644 --- a/eppo_client/eval.py +++ b/eppo_client/eval.py @@ -2,9 +2,7 @@ from eppo_client.sharding import Sharder from eppo_client.models import Range, Shard, Variation from eppo_client.rules import matches_rule -import hashlib from dataclasses import dataclass -import logging @dataclass diff --git a/eppo_client/rules.py b/eppo_client/rules.py index 4a811a4..e5789b0 100644 --- a/eppo_client/rules.py +++ b/eppo_client/rules.py @@ -2,7 +2,7 @@ import re import semver from enum import Enum -from typing import Any, List, Optional, Dict +from typing import Any, List, Dict from eppo_client.models import SdkBaseModel From aed542591f51c30277020389fd1dbe7c3580a3c3 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Mon, 4 Mar 2024 22:43:08 -0800 Subject: [PATCH 04/40] wip --- eppo_client/client.py | 58 +++++++++++----- eppo_client/eval.py | 39 ++++++++--- eppo_client/models.py | 6 +- requirements-test.txt | 3 +- test/client_test.py | 6 -- test/configuration_store_test.py | 30 +++------ test/eval_test.py | 111 ++++++++++++++++++++++++++++++- 7 files changed, 193 insertions(+), 60 deletions(-) diff --git a/eppo_client/client.py b/eppo_client/client.py index 71dca69..c66fb5c 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -12,7 +12,7 @@ from eppo_client.sharding import MD5Sharder from eppo_client.validation import validate_not_blank from eppo_client.variation_type import VariationType -from eppo_client.eval import Evaluator +from eppo_client.eval import FlagEvaluation, Evaluator from eppo_client.models import Variation logger = logging.getLogger(__name__) @@ -37,7 +37,7 @@ def __init__( self.__evaluator = Evaluator(sharder=MD5Sharder()) def get_string_assignment( - self, subject_key: str, flag_key: str, subject_attributes=dict() + self, subject_key: str, flag_key: str, subject_attributes=dict(), default=None ) -> Optional[str]: try: assigned_variation = self.get_assignment_variation( @@ -54,11 +54,15 @@ def get_string_assignment( except Exception as e: if self.__is_graceful_mode: logger.error("[Eppo SDK] Error getting assignment: " + str(e)) - return None + return default raise e def get_numeric_assignment( - self, subject_key: str, flag_key: str, subject_attributes=dict() + self, + subject_key: str, + flag_key: str, + subject_attributes=dict(), + default=None, ) -> Optional[Number]: try: assigned_variation = self.get_assignment_variation( @@ -75,11 +79,15 @@ def get_numeric_assignment( except Exception as e: if self.__is_graceful_mode: logger.error("[Eppo SDK] Error getting assignment: " + str(e)) - return None + return default raise e def get_boolean_assignment( - self, subject_key: str, flag_key: str, subject_attributes=dict() + self, + subject_key: str, + flag_key: str, + subject_attributes=dict(), + default=None, ) -> Optional[bool]: try: assigned_variation = self.get_assignment_variation( @@ -96,11 +104,15 @@ def get_boolean_assignment( except Exception as e: if self.__is_graceful_mode: logger.error("[Eppo SDK] Error getting assignment: " + str(e)) - return None + return default raise e def get_parsed_json_assignment( - self, subject_key: str, flag_key: str, subject_attributes=dict() + self, + subject_key: str, + flag_key: str, + subject_attributes=dict(), + default=None, ) -> Optional[Dict[Any, Any]]: try: assigned_variation = self.get_assignment_variation( @@ -117,13 +129,17 @@ def get_parsed_json_assignment( except Exception as e: if self.__is_graceful_mode: logger.error("[Eppo SDK] Error getting assignment: " + str(e)) - return None + return default raise e def get_json_string_assignment( - self, subject_key: str, flag_key: str, subject_attributes=dict() + self, subject_key: str, flag_key: str, subject_attributes=dict(), default=None ) -> Optional[str]: try: + result = self.get_assignment_detail( + subject_key, flag_key, subject_attributes + ) + assigned_variation = result.variation assigned_variation = self.get_assignment_variation( subject_key, flag_key, subject_attributes, VariationType.JSON ) @@ -138,19 +154,20 @@ def get_json_string_assignment( except Exception as e: if self.__is_graceful_mode: logger.error("[Eppo SDK] Error getting assignment: " + str(e)) - return None + return default raise e @deprecated( "get_assignment is deprecated in favor of the typed get__assignment methods" ) def get_assignment( - self, subject_key: str, flag_key: str, subject_attributes=dict() + self, subject_key: str, flag_key: str, subject_attributes=dict(), default=None ) -> Optional[str]: try: - assigned_variation = self.get_assignment_variation( + result = self.get_assignment_detail( subject_key, flag_key, subject_attributes ) + assigned_variation = result.variation return ( assigned_variation.value if assigned_variation is not None @@ -162,16 +179,16 @@ def get_assignment( except Exception as e: if self.__is_graceful_mode: logger.error("[Eppo SDK] Error getting assignment: " + str(e)) - return None + return default raise e - def get_assignment_variation( + def get_assignment_detail( self, subject_key: str, flag_key: str, subject_attributes: Any, expected_variation_type: Optional[str] = None, - ) -> Optional[Variation]: + ) -> Optional[FlagEvaluation]: """Maps a subject to a variation for a given experiment Returns None if the subject is not part of the experiment sample. @@ -192,6 +209,13 @@ def get_assignment_variation( result = self.__evaluator.evaluate_flag(flag, subject_key, subject_attributes) + if expected_variation_type is not None and result.variation: + variation_is_expected_type = VariationType.is_expected_type( + result.variation, expected_variation_type + ) + if not variation_is_expected_type: + return None + assignment_event = { **result.extra_logging, "allocation": result.allocation_key, @@ -207,7 +231,7 @@ def get_assignment_variation( self.__assignment_logger.log_assignment(assignment_event) except Exception as e: logger.error("[Eppo SDK] Error logging assignment event: " + str(e)) - return result.variation + return result def _shutdown(self): """Stops all background processes used by the client diff --git a/eppo_client/eval.py b/eppo_client/eval.py index 353d057..8419548 100644 --- a/eppo_client/eval.py +++ b/eppo_client/eval.py @@ -3,10 +3,11 @@ from eppo_client.models import Range, Shard, Variation from eppo_client.rules import matches_rule from dataclasses import dataclass +import datetime @dataclass -class EvalResult: +class FlagEvaluation: flag_key: str allocation_key: str variation: Variation @@ -20,9 +21,17 @@ class Evaluator: def evaluate_flag( self, flag, subject_key, subject_attributes - ) -> Optional[EvalResult]: + ) -> Optional[FlagEvaluation]: + if not flag.enabled: + return none_result(flag.key) + + now = utcnow() for allocation in flag.allocations: - # todo: add conditionals for allocation start and end timestamps + if allocation.start_at and now < allocation.start_at: + continue + if allocation.end_at and now > allocation.end_at: + continue + if not allocation.rules or any( matches_rule(rule, subject_attributes) for rule in allocation.rules ): @@ -31,7 +40,7 @@ def evaluate_flag( self.matches_shard(shard, subject_key, flag.total_shards) for shard in split.shards ): - return EvalResult( + return FlagEvaluation( flag_key=flag.key, allocation_key=allocation.key, variation=flag.variations.get(split.variation_key), @@ -39,13 +48,7 @@ def evaluate_flag( do_log=allocation.do_log, ) - return EvalResult( - flag_key=flag.key, - allocation_key=None, - variation=None, - extra_logging={}, - do_log=True, - ) + return none_result(flag.key) def matches_shard(self, shard: Shard, subject_key: str, total_shards: int) -> bool: h = self.sharder.get_shard(seed(shard.salt, subject_key), total_shards) @@ -58,3 +61,17 @@ def is_in_shard_range(shard: int, range: Range) -> bool: def seed(salt, subject_key): return f"{salt}-{subject_key}" + + +def none_result(flag_key): + return FlagEvaluation( + flag_key=flag_key, + allocation_key=None, + variation=None, + extra_logging={}, + do_log=False, + ) + + +def utcnow(): + return datetime.datetime.utcnow() diff --git a/eppo_client/models.py b/eppo_client/models.py index bb8850a..8f3391e 100644 --- a/eppo_client/models.py +++ b/eppo_client/models.py @@ -1,15 +1,15 @@ from datetime import datetime -from typing import Dict, List, Optional +from typing import Any, Dict, List, Optional from eppo_client.base_model import SdkBaseModel from eppo_client.rules import Rule class Variation(SdkBaseModel): - # TODO: generalize key: str - value: str + value: str | int | float | bool + typed_value: Any = None class Range(SdkBaseModel): diff --git a/requirements-test.txt b/requirements-test.txt index 7f2a005..c66caf9 100644 --- a/requirements-test.txt +++ b/requirements-test.txt @@ -1,4 +1,5 @@ tox pytest +pytest-mock mypy -httpretty \ No newline at end of file +httpretty diff --git a/test/client_test.py b/test/client_test.py index 36d970e..9d50154 100644 --- a/test/client_test.py +++ b/test/client_test.py @@ -7,13 +7,7 @@ from eppo_client.assignment_logger import AssignmentLogger from eppo_client.client import EppoClient from eppo_client.config import Config -from eppo_client.configuration_requestor import ( - AllocationDto, - ExperimentConfigurationDto, - VariationDto, -) from eppo_client.rules import Condition, OperatorType, Rule -from eppo_client.shard import ShardRange from eppo_client import init, get_instance test_data = [] diff --git a/test/configuration_store_test.py b/test/configuration_store_test.py index 40c4beb..5b8ca30 100644 --- a/test/configuration_store_test.py +++ b/test/configuration_store_test.py @@ -1,41 +1,33 @@ -from eppo_client.configuration_requestor import ( - AllocationDto, - ExperimentConfigurationDto, -) +from eppo_client.models import Flag from eppo_client.configuration_store import ConfigurationStore -test_exp = ExperimentConfigurationDto( - subject_shards=1000, - enabled=True, - name="randomization_algo", - allocations={"allocation-1": AllocationDto(percent_exposure=1, variations=[])}, -) TEST_MAX_SIZE = 10 -store: ConfigurationStore[ExperimentConfigurationDto] = ConfigurationStore( - max_size=TEST_MAX_SIZE +store: ConfigurationStore[str] = ConfigurationStore(max_size=TEST_MAX_SIZE) +mock_flag = Flag( + key="mock_flag", enabled=True, variations={}, allocations=[], total_shards=10000 ) def test_get_configuration_unknown_key(): - store.set_configurations({"randomization_algo": test_exp}) + store.set_configurations({"flag": mock_flag}) assert store.get_configuration("unknown_exp") is None def test_get_configuration_known_key(): - store.set_configurations({"randomization_algo": test_exp}) - assert store.get_configuration("randomization_algo") == test_exp + store.set_configurations({"flag": mock_flag}) + assert store.get_configuration("flag") == mock_flag def test_evicts_old_entries_when_max_size_exceeded(): - store.set_configurations({"item_to_be_evicted": test_exp}) - assert store.get_configuration("item_to_be_evicted") == test_exp + store.set_configurations({"item_to_be_evicted": mock_flag}) + assert store.get_configuration("item_to_be_evicted") == mock_flag configs = {} for i in range(0, TEST_MAX_SIZE): - configs["test-entry-{}".format(i)] = test_exp + configs["test-entry-{}".format(i)] = mock_flag store.set_configurations(configs) assert store.get_configuration("item_to_be_evicted") is None assert ( - store.get_configuration("test-entry-{}".format(TEST_MAX_SIZE - 1)) == test_exp + store.get_configuration("test-entry-{}".format(TEST_MAX_SIZE - 1)) == mock_flag ) diff --git a/test/eval_test.py b/test/eval_test.py index 5a087b8..24846ed 100644 --- a/test/eval_test.py +++ b/test/eval_test.py @@ -1,9 +1,32 @@ +import datetime + from eppo_client.models import Flag, Allocation, Range, Variation, Split, Shard -from eppo_client.eval import Evaluator, EvalResult +from eppo_client.eval import Evaluator, FlagEvaluation from eppo_client.rules import Condition, OperatorType, Rule from eppo_client.sharding import DeterministicSharder, MD5Sharder +def test_disabled_flag_returns_none_result(): + flag = Flag( + key="disabled_flag", + enabled=False, + variations={"a": Variation(key="a", value="A")}, + allocations=[ + Allocation( + key="default", rules=[], splits=[Split(variation_key="a", shards=[])] + ) + ], + total_shards=10, + ) + + evaluator = Evaluator(sharder=MD5Sharder()) + result = evaluator.evaluate_flag(flag, "subject_key", {}) + assert result.flag_key == "disabled_flag" + assert result.allocation_key == None + assert result.variation == None + assert not result.do_log + + def test_matches_shard_full_range(): shard = Shard( salt="a", @@ -47,12 +70,12 @@ def test_eval_empty_flag(): ) evaluator = Evaluator(sharder=MD5Sharder()) - assert evaluator.evaluate_flag(empty_flag, "subject_key", {}) == EvalResult( + assert evaluator.evaluate_flag(empty_flag, "subject_key", {}) == FlagEvaluation( flag_key="empty", allocation_key=None, variation=None, extra_logging={}, - do_log=True, + do_log=False, ) @@ -79,6 +102,7 @@ def test_catch_all_allocation(): assert result.flag_key == "flag" assert result.allocation_key == "default" assert result.variation == Variation(key="a", value="A") + assert result.do_log def test_match_first_allocation_rule(): @@ -232,3 +256,84 @@ def test_eval_sharding(): result = evaluator.evaluate_flag(flag, "dave", {}) assert result.allocation_key == "default" assert result.variation == Variation(key="c", value="C") + + +def test_eval_prior_to_alloc(mocker): + flag = Flag( + key="flag", + enabled=True, + variations={"a": Variation(key="a", value="A")}, + allocations=[ + Allocation( + key="default", + start_at=datetime.datetime(2024, 1, 1), + end_at=datetime.datetime(2024, 2, 1), + rules=[], + splits=[Split(variation_key="a", shards=[])], + ) + ], + total_shards=10, + ) + + evaluator = Evaluator(sharder=MD5Sharder()) + with mocker.patch( + "eppo_client.eval.utcnow", return_value=datetime.datetime(2023, 1, 1) + ): + result = evaluator.evaluate_flag(flag, "subject_key", {}) + assert result.flag_key == "flag" + assert result.allocation_key == None + assert result.variation == None + + +def test_eval_during_alloc(mocker): + flag = Flag( + key="flag", + enabled=True, + variations={"a": Variation(key="a", value="A")}, + allocations=[ + Allocation( + key="default", + start_at=datetime.datetime(2024, 1, 1), + end_at=datetime.datetime(2024, 2, 1), + rules=[], + splits=[Split(variation_key="a", shards=[])], + ) + ], + total_shards=10, + ) + + evaluator = Evaluator(sharder=MD5Sharder()) + with mocker.patch( + "eppo_client.eval.utcnow", return_value=datetime.datetime(2024, 1, 5) + ): + result = evaluator.evaluate_flag(flag, "subject_key", {}) + assert result.flag_key == "flag" + assert result.allocation_key == "default" + assert result.variation == Variation(key="a", value="A") + + +def test_eval_after_alloc(mocker): + flag = Flag( + key="flag", + enabled=True, + variations={"a": Variation(key="a", value="A")}, + allocations=[ + Allocation( + key="default", + start_at=datetime.datetime(2024, 1, 1), + end_at=datetime.datetime(2024, 2, 1), + rules=[], + splits=[Split(variation_key="a", shards=[])], + ) + ], + total_shards=10, + ) + + evaluator = Evaluator(sharder=MD5Sharder()) + with mocker.patch( + "eppo_client.eval.utcnow", return_value=datetime.datetime(2024, 2, 5) + ): + result = evaluator.evaluate_flag(flag, "subject_key", {}) + assert result.flag_key == "flag" + assert result.allocation_key == None + assert result.variation == None From 8ba333086e13b4469326889c54b116f5a4ee4d02 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Mon, 4 Mar 2024 22:56:09 -0800 Subject: [PATCH 05/40] wip --- eppo_client/eval.py | 18 +++++++++++++++--- test/eval_test.py | 18 +++++++++++++++++- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/eppo_client/eval.py b/eppo_client/eval.py index 8419548..d521b67 100644 --- a/eppo_client/eval.py +++ b/eppo_client/eval.py @@ -9,6 +9,8 @@ @dataclass class FlagEvaluation: flag_key: str + subject_key: str + subject_attributes: Dict[str, str] allocation_key: str variation: Variation extra_logging: Dict[str, str] @@ -23,32 +25,40 @@ def evaluate_flag( self, flag, subject_key, subject_attributes ) -> Optional[FlagEvaluation]: if not flag.enabled: - return none_result(flag.key) + return none_result(flag.key, subject_key, subject_attributes) now = utcnow() for allocation in flag.allocations: + # Skip allocations that are not active if allocation.start_at and now < allocation.start_at: continue if allocation.end_at and now > allocation.end_at: continue + # Skip allocations when none of the rules match + # So we look for (rule 1) OR (rule 2) OR (rule 3) etc. + # If there are no rules, then we always match if not allocation.rules or any( matches_rule(rule, subject_attributes) for rule in allocation.rules ): for split in allocation.splits: + # Split needs to match all shards if all( self.matches_shard(shard, subject_key, flag.total_shards) for shard in split.shards ): return FlagEvaluation( flag_key=flag.key, + subject_key=subject_key, + subject_attributes=subject_attributes, allocation_key=allocation.key, variation=flag.variations.get(split.variation_key), extra_logging=split.extra_logging, do_log=allocation.do_log, ) - return none_result(flag.key) + # No allocations matched, return the None result + return none_result(flag.key, subject_key, subject_attributes) def matches_shard(self, shard: Shard, subject_key: str, total_shards: int) -> bool: h = self.sharder.get_shard(seed(shard.salt, subject_key), total_shards) @@ -63,9 +73,11 @@ def seed(salt, subject_key): return f"{salt}-{subject_key}" -def none_result(flag_key): +def none_result(flag_key, subject_key, subject_attributes): return FlagEvaluation( flag_key=flag_key, + subject_key=subject_key, + subject_attributes=subject_attributes, allocation_key=None, variation=None, extra_logging={}, diff --git a/test/eval_test.py b/test/eval_test.py index 24846ed..1af2bf1 100644 --- a/test/eval_test.py +++ b/test/eval_test.py @@ -1,7 +1,7 @@ import datetime from eppo_client.models import Flag, Allocation, Range, Variation, Split, Shard -from eppo_client.eval import Evaluator, FlagEvaluation +from eppo_client.eval import Evaluator, FlagEvaluation, is_in_shard_range, seed from eppo_client.rules import Condition, OperatorType, Rule from eppo_client.sharding import DeterministicSharder, MD5Sharder @@ -72,6 +72,8 @@ def test_eval_empty_flag(): evaluator = Evaluator(sharder=MD5Sharder()) assert evaluator.evaluate_flag(empty_flag, "subject_key", {}) == FlagEvaluation( flag_key="empty", + subject_key="subject_key", + subject_attributes={}, allocation_key=None, variation=None, extra_logging={}, @@ -337,3 +339,17 @@ def test_eval_after_alloc(mocker): assert result.flag_key == "flag" assert result.allocation_key == None assert result.variation == None + + +def test_seed(): + assert seed("salt", "subject") == "salt-subject" + + +def test_is_in_shard_range(): + assert is_in_shard_range(5, Range(start=0, end=10)) is True + assert is_in_shard_range(10, Range(start=0, end=10)) is False + assert is_in_shard_range(0, Range(start=0, end=10)) is True + assert is_in_shard_range(0, Range(start=0, end=0)) is False + assert is_in_shard_range(0, Range(start=0, end=1)) is True + assert is_in_shard_range(1, Range(start=0, end=1)) is False + assert is_in_shard_range(1, Range(start=1, end=1)) is False From 26006ec51315150d41ce208ba76d6ec80e01d0f1 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Tue, 5 Mar 2024 10:24:13 -0800 Subject: [PATCH 06/40] wip --- eppo_client/client.py | 1 - eppo_client/eval.py | 6 +++--- test/eval_test.py | 4 ++-- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/eppo_client/client.py b/eppo_client/client.py index c66fb5c..511749d 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -13,7 +13,6 @@ from eppo_client.validation import validate_not_blank from eppo_client.variation_type import VariationType from eppo_client.eval import FlagEvaluation, Evaluator -from eppo_client.models import Variation logger = logging.getLogger(__name__) diff --git a/eppo_client/eval.py b/eppo_client/eval.py index d521b67..ece50f5 100644 --- a/eppo_client/eval.py +++ b/eppo_client/eval.py @@ -10,7 +10,7 @@ class FlagEvaluation: flag_key: str subject_key: str - subject_attributes: Dict[str, str] + subject_attributes: Dict[str, str | int | float | bool] allocation_key: str variation: Variation extra_logging: Dict[str, str] @@ -61,7 +61,7 @@ def evaluate_flag( return none_result(flag.key, subject_key, subject_attributes) def matches_shard(self, shard: Shard, subject_key: str, total_shards: int) -> bool: - h = self.sharder.get_shard(seed(shard.salt, subject_key), total_shards) + h = self.sharder.get_shard(hash_key(shard.salt, subject_key), total_shards) return any(is_in_shard_range(h, r) for r in shard.ranges) @@ -69,7 +69,7 @@ def is_in_shard_range(shard: int, range: Range) -> bool: return range.start <= shard < range.end -def seed(salt, subject_key): +def hash_key(salt, subject_key): return f"{salt}-{subject_key}" diff --git a/test/eval_test.py b/test/eval_test.py index 1af2bf1..574f366 100644 --- a/test/eval_test.py +++ b/test/eval_test.py @@ -1,7 +1,7 @@ import datetime from eppo_client.models import Flag, Allocation, Range, Variation, Split, Shard -from eppo_client.eval import Evaluator, FlagEvaluation, is_in_shard_range, seed +from eppo_client.eval import Evaluator, FlagEvaluation, is_in_shard_range, hash_key from eppo_client.rules import Condition, OperatorType, Rule from eppo_client.sharding import DeterministicSharder, MD5Sharder @@ -342,7 +342,7 @@ def test_eval_after_alloc(mocker): def test_seed(): - assert seed("salt", "subject") == "salt-subject" + assert hash_key("salt", "subject") == "salt-subject" def test_is_in_shard_range(): From 36a3e1815dea535ab0cdbd1784e31e8ccb8a311a Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Wed, 6 Mar 2024 14:48:51 -0800 Subject: [PATCH 07/40] address comments --- eppo_client/client.py | 30 +++++++++++++++++++++++------- eppo_client/config.py | 2 +- eppo_client/rules.py | 11 +++++++---- 3 files changed, 31 insertions(+), 12 deletions(-) diff --git a/eppo_client/client.py b/eppo_client/client.py index 511749d..efa42f9 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -9,6 +9,7 @@ ) from eppo_client.constants import POLL_INTERVAL_MILLIS, POLL_JITTER_MILLIS from eppo_client.poller import Poller +from eppo_client.rules import AttributeValue, SubjectAttributes from eppo_client.sharding import MD5Sharder from eppo_client.validation import validate_not_blank from eppo_client.variation_type import VariationType @@ -36,7 +37,11 @@ def __init__( self.__evaluator = Evaluator(sharder=MD5Sharder()) def get_string_assignment( - self, subject_key: str, flag_key: str, subject_attributes=dict(), default=None + self, + subject_key: str, + flag_key: str, + subject_attributes: Optional[SubjectAttributes] = None, + default=None, ) -> Optional[str]: try: assigned_variation = self.get_assignment_variation( @@ -60,7 +65,7 @@ def get_numeric_assignment( self, subject_key: str, flag_key: str, - subject_attributes=dict(), + subject_attributes: Optional[SubjectAttributes] = None, default=None, ) -> Optional[Number]: try: @@ -85,7 +90,7 @@ def get_boolean_assignment( self, subject_key: str, flag_key: str, - subject_attributes=dict(), + subject_attributes: Optional[SubjectAttributes] = None, default=None, ) -> Optional[bool]: try: @@ -110,7 +115,7 @@ def get_parsed_json_assignment( self, subject_key: str, flag_key: str, - subject_attributes=dict(), + subject_attributes: Optional[SubjectAttributes] = None, default=None, ) -> Optional[Dict[Any, Any]]: try: @@ -132,7 +137,11 @@ def get_parsed_json_assignment( raise e def get_json_string_assignment( - self, subject_key: str, flag_key: str, subject_attributes=dict(), default=None + self, + subject_key: str, + flag_key: str, + subject_attributes: Optional[SubjectAttributes] = None, + default=None, ) -> Optional[str]: try: result = self.get_assignment_detail( @@ -160,7 +169,11 @@ def get_json_string_assignment( "get_assignment is deprecated in favor of the typed get__assignment methods" ) def get_assignment( - self, subject_key: str, flag_key: str, subject_attributes=dict(), default=None + self, + subject_key: str, + flag_key: str, + subject_attributes: Optional[SubjectAttributes] = None, + default=None, ) -> Optional[str]: try: result = self.get_assignment_detail( @@ -185,7 +198,7 @@ def get_assignment_detail( self, subject_key: str, flag_key: str, - subject_attributes: Any, + subject_attributes: Optional[SubjectAttributes] = None, expected_variation_type: Optional[str] = None, ) -> Optional[FlagEvaluation]: """Maps a subject to a variation for a given experiment @@ -198,6 +211,9 @@ def get_assignment_detail( """ validate_not_blank("subject_key", subject_key) validate_not_blank("flag_key", flag_key) + if not subject_attributes: + subject_attributes = {} + flag = self.__config_requestor.get_configuration(flag_key) if flag is None or not flag.enabled: diff --git a/eppo_client/config.py b/eppo_client/config.py index 17a4106..d9d7d88 100644 --- a/eppo_client/config.py +++ b/eppo_client/config.py @@ -1,5 +1,5 @@ from eppo_client.assignment_logger import AssignmentLogger -from eppo_client.models import SdkBaseModel +from eppo_client.base_model import SdkBaseModel from eppo_client.validation import validate_not_blank diff --git a/eppo_client/rules.py b/eppo_client/rules.py index e5789b0..f9046dd 100644 --- a/eppo_client/rules.py +++ b/eppo_client/rules.py @@ -2,10 +2,13 @@ import re import semver from enum import Enum -from typing import Any, List, Dict +from typing import Any, List, Dict, TypeAlias from eppo_client.models import SdkBaseModel +AttributeValue: TypeAlias = str | int | float | bool +SubjectAttributes: TypeAlias = Dict[str, AttributeValue] + class OperatorType(Enum): MATCHES = "MATCHES" @@ -19,7 +22,7 @@ class OperatorType(Enum): class Condition(SdkBaseModel): operator: OperatorType - attribute: str + attribute: AttributeValue value: Any = None @@ -27,7 +30,7 @@ class Rule(SdkBaseModel): conditions: List[Condition] -def matches_rule(rule: Rule, subject_attributes: Dict[str, str]) -> bool: +def matches_rule(rule: Rule, subject_attributes: SubjectAttributes) -> bool: return all( evaluate_condition(condition, subject_attributes) for condition in rule.conditions @@ -35,7 +38,7 @@ def matches_rule(rule: Rule, subject_attributes: Dict[str, str]) -> bool: def evaluate_condition( - condition: Condition, subject_attributes: Dict[str, str] + condition: Condition, subject_attributes: SubjectAttributes ) -> bool: subject_value = subject_attributes.get(condition.attribute, None) if subject_value is not None: From 0df2809b2999d18912935f90d56254ff0b841aa8 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Wed, 13 Mar 2024 20:47:39 -0700 Subject: [PATCH 08/40] wip --- eppo_client/client.py | 218 ++++++++---------- eppo_client/configuration_requestor.py | 15 +- eppo_client/configuration_store.py | 3 + eppo_client/eval.py | 11 +- eppo_client/http_client.py | 2 +- eppo_client/models.py | 15 +- eppo_client/read_write_lock.py | 2 +- eppo_client/rules.py | 13 +- eppo_client/variation_type.py | 31 --- test/client_test.py | 296 +++++++++---------------- test/eval_test.py | 125 +++++++---- 11 files changed, 326 insertions(+), 405 deletions(-) delete mode 100644 eppo_client/variation_type.py diff --git a/eppo_client/client.py b/eppo_client/client.py index efa42f9..141ebca 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -1,18 +1,16 @@ import datetime import logging -from typing import Any, Dict, Optional +from typing import Any, Dict, Optional, Union from typing_extensions import deprecated -from numbers import Number from eppo_client.assignment_logger import AssignmentLogger from eppo_client.configuration_requestor import ( ExperimentConfigurationRequestor, ) from eppo_client.constants import POLL_INTERVAL_MILLIS, POLL_JITTER_MILLIS +from eppo_client.models import ValueType from eppo_client.poller import Poller -from eppo_client.rules import AttributeValue, SubjectAttributes from eppo_client.sharding import MD5Sharder from eppo_client.validation import validate_not_blank -from eppo_client.variation_type import VariationType from eppo_client.eval import FlagEvaluation, Evaluator logger = logging.getLogger(__name__) @@ -40,151 +38,128 @@ def get_string_assignment( self, subject_key: str, flag_key: str, - subject_attributes: Optional[SubjectAttributes] = None, + subject_attributes: Optional[Dict[str, Union[str, float, int, bool]]] = None, default=None, ) -> Optional[str]: - try: - assigned_variation = self.get_assignment_variation( - subject_key, flag_key, subject_attributes, VariationType.STRING - ) - return ( - assigned_variation.typed_value - if assigned_variation is not None - else assigned_variation - ) - except ValueError as e: - # allow ValueError to bubble up as it is a validation error - raise e - except Exception as e: - if self.__is_graceful_mode: - logger.error("[Eppo SDK] Error getting assignment: " + str(e)) - return default - raise e + return self.get_assignment_variation( + subject_key, + flag_key, + subject_attributes, + ValueType.STRING, + default=default, + ) + + def get_integer_assignment( + self, + subject_key: str, + flag_key: str, + subject_attributes: Optional[Dict[str, Union[str, float, int, bool]]] = None, + default=None, + ) -> Optional[float]: + return self.get_assignment_variation( + subject_key, + flag_key, + subject_attributes, + ValueType.INTEGER, + default=default, + ) + + def get_float_assignment( + self, + subject_key: str, + flag_key: str, + subject_attributes: Optional[Dict[str, Union[str, float, int, bool]]] = None, + default=None, + ) -> Optional[float]: + return self.get_assignment_variation( + subject_key, + flag_key, + subject_attributes, + ValueType.FLOAT, + default=default, + ) + @deprecated("get_numeric_assignment is deprecated in favor of get_float_assignment") def get_numeric_assignment( self, subject_key: str, flag_key: str, - subject_attributes: Optional[SubjectAttributes] = None, + subject_attributes: Optional[Dict[str, Union[str, float, int, bool]]] = None, default=None, - ) -> Optional[Number]: - try: - assigned_variation = self.get_assignment_variation( - subject_key, flag_key, subject_attributes, VariationType.NUMERIC - ) - return ( - assigned_variation.typed_value - if assigned_variation is not None - else assigned_variation - ) - except ValueError as e: - # allow ValueError to bubble up as it is a validation error - raise e - except Exception as e: - if self.__is_graceful_mode: - logger.error("[Eppo SDK] Error getting assignment: " + str(e)) - return default - raise e + ) -> Optional[float]: + return self.get_float_assignment( + subject_key, + flag_key, + subject_attributes, + default=default, + ) def get_boolean_assignment( self, subject_key: str, flag_key: str, - subject_attributes: Optional[SubjectAttributes] = None, + subject_attributes: Optional[Dict[str, Union[str, float, int, bool]]] = None, default=None, ) -> Optional[bool]: - try: - assigned_variation = self.get_assignment_variation( - subject_key, flag_key, subject_attributes, VariationType.BOOLEAN - ) - return ( - assigned_variation.typed_value - if assigned_variation is not None - else assigned_variation - ) - except ValueError as e: - # allow ValueError to bubble up as it is a validation error - raise e - except Exception as e: - if self.__is_graceful_mode: - logger.error("[Eppo SDK] Error getting assignment: " + str(e)) - return default - raise e + return self.get_assignment_variation( + subject_key, + flag_key, + subject_attributes, + ValueType.BOOLEAN, + default=default, + ) def get_parsed_json_assignment( self, subject_key: str, flag_key: str, - subject_attributes: Optional[SubjectAttributes] = None, + subject_attributes: Optional[Dict[str, Union[str, float, int, bool]]] = None, default=None, ) -> Optional[Dict[Any, Any]]: - try: - assigned_variation = self.get_assignment_variation( - subject_key, flag_key, subject_attributes, VariationType.JSON - ) - return ( - assigned_variation.typed_value - if assigned_variation is not None - else assigned_variation - ) - except ValueError as e: - # allow ValueError to bubble up as it is a validation error - raise e - except Exception as e: - if self.__is_graceful_mode: - logger.error("[Eppo SDK] Error getting assignment: " + str(e)) - return default - raise e + return self.get_assignment_variation( + subject_key, + flag_key, + subject_attributes, + ValueType.JSON, + default=default, + ) - def get_json_string_assignment( + @deprecated( + "get_assignment is deprecated in favor of the typed get__assignment methods" + ) + def get_assignment( self, subject_key: str, flag_key: str, - subject_attributes: Optional[SubjectAttributes] = None, + subject_attributes: Optional[Dict[str, Union[str, float, int, bool]]] = None, default=None, ) -> Optional[str]: - try: - result = self.get_assignment_detail( - subject_key, flag_key, subject_attributes - ) - assigned_variation = result.variation - assigned_variation = self.get_assignment_variation( - subject_key, flag_key, subject_attributes, VariationType.JSON - ) - return ( - assigned_variation.value - if assigned_variation is not None - else assigned_variation - ) - except ValueError as e: - # allow ValueError to bubble up as it is a validation error - raise e - except Exception as e: - if self.__is_graceful_mode: - logger.error("[Eppo SDK] Error getting assignment: " + str(e)) - return default - raise e + return self.get_assignment_variation( + subject_key, flag_key, subject_attributes, default=default + ) - @deprecated( - "get_assignment is deprecated in favor of the typed get__assignment methods" - ) - def get_assignment( + def get_assignment_variation( self, subject_key: str, flag_key: str, - subject_attributes: Optional[SubjectAttributes] = None, + subject_attributes: Optional[Dict[str, Union[str, float, int, bool]]] = None, + expected_variation_type: Optional[ValueType] = None, default=None, - ) -> Optional[str]: + ): try: result = self.get_assignment_detail( subject_key, flag_key, subject_attributes ) + if not result: + return default assigned_variation = result.variation - return ( - assigned_variation.value - if assigned_variation is not None - else assigned_variation - ) + if not check_type_match( + assigned_variation.value_type, expected_variation_type + ): + raise TypeError( + "Variation value does not have the correct type. Found: {assigned_variation.value_type} != {expected_variation_type}" + ) + return assigned_variation.value except ValueError as e: # allow ValueError to bubble up as it is a validation error raise e @@ -198,8 +173,7 @@ def get_assignment_detail( self, subject_key: str, flag_key: str, - subject_attributes: Optional[SubjectAttributes] = None, - expected_variation_type: Optional[str] = None, + subject_attributes: Optional[Dict[str, Union[str, float, int, bool]]] = None, ) -> Optional[FlagEvaluation]: """Maps a subject to a variation for a given experiment Returns None if the subject is not part of the experiment sample. @@ -224,13 +198,6 @@ def get_assignment_detail( result = self.__evaluator.evaluate_flag(flag, subject_key, subject_attributes) - if expected_variation_type is not None and result.variation: - variation_is_expected_type = VariationType.is_expected_type( - result.variation, expected_variation_type - ) - if not variation_is_expected_type: - return None - assignment_event = { **result.extra_logging, "allocation": result.allocation_key, @@ -248,8 +215,19 @@ def get_assignment_detail( logger.error("[Eppo SDK] Error logging assignment event: " + str(e)) return result + def get_flag_keys(self): + return self.__config_requestor.get_flag_keys() + def _shutdown(self): """Stops all background processes used by the client Do not use the client after calling this method. """ self.__poller.stop() + + +def check_type_match(value_type, expected_type): + return ( + expected_type is None + or value_type == expected_type + or value_type == expected_type.value + ) diff --git a/eppo_client/configuration_requestor.py b/eppo_client/configuration_requestor.py index 6592e39..9f468f6 100644 --- a/eppo_client/configuration_requestor.py +++ b/eppo_client/configuration_requestor.py @@ -7,7 +7,7 @@ logger = logging.getLogger(__name__) -RAC_ENDPOINT = "/randomized_assignment/v3/config" +UFC_ENDPOINT = "/flag_config/v1/config" class ExperimentConfigurationRequestor: @@ -19,16 +19,19 @@ def __init__( self.__http_client = http_client self.__config_store = config_store - def get_configuration(self, experiment_key: str) -> Optional[Flag]: + def get_configuration(self, flag_key: str) -> Optional[Flag]: if self.__http_client.is_unauthorized(): raise ValueError("Unauthorized: please check your API key") - return self.__config_store.get_configuration(experiment_key) + return self.__config_store.get_configuration(flag_key) + + def get_flag_keys(self): + return self.__config_store.get_keys() def fetch_and_store_configurations(self) -> Dict[str, Flag]: try: - configs = cast(dict, self.__http_client.get(RAC_ENDPOINT).get("flags", {})) - for exp_key, exp_config in configs.items(): - configs[exp_key] = Flag(**exp_config) + configs = cast(dict, self.__http_client.get(UFC_ENDPOINT).get("flags", {})) + for flag_key, flag_config in configs.items(): + configs[flag_key] = Flag(**flag_config) self.__config_store.set_configurations(configs) return configs except Exception as e: diff --git a/eppo_client/configuration_store.py b/eppo_client/configuration_store.py index f9c57f4..b54ff7c 100644 --- a/eppo_client/configuration_store.py +++ b/eppo_client/configuration_store.py @@ -27,3 +27,6 @@ def set_configurations(self, configs: Dict[str, T]): self.__cache[key] = config finally: self.__lock.release_write() + + def get_keys(self): + return list(self.__cache.keys()) diff --git a/eppo_client/eval.py b/eppo_client/eval.py index ece50f5..eef9d84 100644 --- a/eppo_client/eval.py +++ b/eppo_client/eval.py @@ -1,6 +1,6 @@ -from typing import Dict, Optional +from typing import Dict, Optional, Union from eppo_client.sharding import Sharder -from eppo_client.models import Range, Shard, Variation +from eppo_client.models import Flag, Range, Shard, Variation from eppo_client.rules import matches_rule from dataclasses import dataclass import datetime @@ -10,7 +10,7 @@ class FlagEvaluation: flag_key: str subject_key: str - subject_attributes: Dict[str, str | int | float | bool] + subject_attributes: Dict[str, Union[str, float, int, bool]] allocation_key: str variation: Variation extra_logging: Dict[str, str] @@ -22,7 +22,10 @@ class Evaluator: sharder: Sharder def evaluate_flag( - self, flag, subject_key, subject_attributes + self, + flag: Flag, + subject_key: str, + subject_attributes: Dict[str, Union[str, float, int, bool]], ) -> Optional[FlagEvaluation]: if not flag.enabled: return none_result(flag.key, subject_key, subject_attributes) diff --git a/eppo_client/http_client.py b/eppo_client/http_client.py index 388b074..8ef4a1c 100644 --- a/eppo_client/http_client.py +++ b/eppo_client/http_client.py @@ -43,7 +43,7 @@ def get(self, resource: str) -> Any: try: response = self.__session.get( self.__base_url + resource, - params=self.__sdk_params.dict(), + params=self.__sdk_params.model_dump(), timeout=REQUEST_TIMEOUT_SECONDS, ) self.__is_unauthorized = response.status_code == HTTPStatus.UNAUTHORIZED diff --git a/eppo_client/models.py b/eppo_client/models.py index 8f3391e..abdb704 100644 --- a/eppo_client/models.py +++ b/eppo_client/models.py @@ -1,15 +1,24 @@ from datetime import datetime +from enum import Enum -from typing import Any, Dict, List, Optional +from typing import Dict, List, Optional, Union from eppo_client.base_model import SdkBaseModel from eppo_client.rules import Rule +class ValueType(Enum): + STRING = "string" + INTEGER = "integer" + FLOAT = "float" + BOOLEAN = "boolean" + JSON = "json" + + class Variation(SdkBaseModel): key: str - value: str | int | float | bool - typed_value: Any = None + value: Union[str, int, float, bool] + value_type: ValueType class Range(SdkBaseModel): diff --git a/eppo_client/read_write_lock.py b/eppo_client/read_write_lock.py index 7f0c5fe..ef03431 100644 --- a/eppo_client/read_write_lock.py +++ b/eppo_client/read_write_lock.py @@ -26,7 +26,7 @@ def release_read(self): try: self._readers -= 1 if not self._readers: - self._read_ready.notifyAll() + self._read_ready.notify_all() finally: self._read_ready.release() diff --git a/eppo_client/rules.py b/eppo_client/rules.py index f9046dd..41dae8e 100644 --- a/eppo_client/rules.py +++ b/eppo_client/rules.py @@ -2,13 +2,10 @@ import re import semver from enum import Enum -from typing import Any, List, Dict, TypeAlias +from typing import Any, List, Dict, Union from eppo_client.models import SdkBaseModel -AttributeValue: TypeAlias = str | int | float | bool -SubjectAttributes: TypeAlias = Dict[str, AttributeValue] - class OperatorType(Enum): MATCHES = "MATCHES" @@ -22,7 +19,7 @@ class OperatorType(Enum): class Condition(SdkBaseModel): operator: OperatorType - attribute: AttributeValue + attribute: Any value: Any = None @@ -30,7 +27,9 @@ class Rule(SdkBaseModel): conditions: List[Condition] -def matches_rule(rule: Rule, subject_attributes: SubjectAttributes) -> bool: +def matches_rule( + rule: Rule, subject_attributes: Dict[str, Union[str, float, int, bool]] +) -> bool: return all( evaluate_condition(condition, subject_attributes) for condition in rule.conditions @@ -38,7 +37,7 @@ def matches_rule(rule: Rule, subject_attributes: SubjectAttributes) -> bool: def evaluate_condition( - condition: Condition, subject_attributes: SubjectAttributes + condition: Condition, subject_attributes: Dict[str, Union[str, float, int, bool]] ) -> bool: subject_value = subject_attributes.get(condition.attribute, None) if subject_value is not None: diff --git a/eppo_client/variation_type.py b/eppo_client/variation_type.py deleted file mode 100644 index 29725a7..0000000 --- a/eppo_client/variation_type.py +++ /dev/null @@ -1,31 +0,0 @@ -import json -from numbers import Number -from eppo_client.models import Variation - - -class VariationType: - STRING = "string" - NUMERIC = "numeric" - BOOLEAN = "boolean" - JSON = "json" - - @classmethod - def is_expected_type( - cls, assigned_variation: Variation, expected_variation_type: str - ) -> bool: - if expected_variation_type == cls.STRING: - return isinstance(assigned_variation.typed_value, str) - elif expected_variation_type == cls.NUMERIC: - return isinstance( - assigned_variation.typed_value, Number - ) and not isinstance(assigned_variation.typed_value, bool) - elif expected_variation_type == cls.BOOLEAN: - return isinstance(assigned_variation.typed_value, bool) - elif expected_variation_type == cls.JSON: - try: - parsed_json = json.loads(assigned_variation.value) - json.dumps(assigned_variation.typed_value) - return parsed_json == assigned_variation.typed_value - except (json.JSONDecodeError, TypeError): - pass - return False diff --git a/test/client_test.py b/test/client_test.py index 9d50154..da0bbdd 100644 --- a/test/client_test.py +++ b/test/client_test.py @@ -5,14 +5,29 @@ import httpretty # type: ignore import pytest from eppo_client.assignment_logger import AssignmentLogger -from eppo_client.client import EppoClient +from eppo_client.client import EppoClient, check_type_match from eppo_client.config import Config +from eppo_client.models import ( + Allocation, + Flag, + Range, + Shard, + Split, + ValueType, + Variation, +) from eppo_client.rules import Condition, OperatorType, Rule from eppo_client import init, get_instance +import logging + +logger = logging.getLogger(__name__) + +TEST_DIR = "test/test-data/ufc/tests" +CONFIG_FILE = "test/test-data/ufc/flags-v1.json" test_data = [] -for file_name in [file for file in os.listdir("test/test-data/assignment-v2")]: - with open("test/test-data/assignment-v2/{}".format(file_name)) as test_case_json: +for file_name in [file for file in os.listdir(TEST_DIR)]: + with open("{}/{}".format(TEST_DIR, file_name)) as test_case_json: test_case_dict = json.load(test_case_json) test_data.append(test_case_dict) @@ -22,11 +37,11 @@ @pytest.fixture(scope="session", autouse=True) def init_fixture(): httpretty.enable() - with open("test/test-data/rac-experiments-v3.json") as mock_rac_response: + with open(CONFIG_FILE) as mock_ufc_response: httpretty.register_uri( httpretty.GET, - MOCK_BASE_URL + "/randomized_assignment/v3/config", - body=json.dumps(json.load(mock_rac_response)), + MOCK_BASE_URL + "/flag_config/v1/config", + body=json.dumps(json.load(mock_ufc_response)), ) client = init( Config( @@ -42,12 +57,12 @@ def init_fixture(): @patch("eppo_client.configuration_requestor.ExperimentConfigurationRequestor") -def test_assign_blank_experiment(mock_config_requestor): +def test_assign_blank_flag_key(mock_config_requestor): client = EppoClient( config_requestor=mock_config_requestor, assignment_logger=AssignmentLogger() ) with pytest.raises(Exception) as exc_info: - client.get_assignment("subject-1", "") + client.get_string_assignment("subject-1", "") assert exc_info.value.args[0] == "Invalid value for flag_key: cannot be blank" @@ -57,184 +72,77 @@ def test_assign_blank_subject(mock_config_requestor): config_requestor=mock_config_requestor, assignment_logger=AssignmentLogger() ) with pytest.raises(Exception) as exc_info: - client.get_assignment("", "experiment-1") + client.get_string_assignment("", "experiment-1") assert exc_info.value.args[0] == "Invalid value for subject_key: cannot be blank" -@patch("eppo_client.configuration_requestor.ExperimentConfigurationRequestor") -def test_assign_subject_not_in_sample(mock_config_requestor): - allocation = AllocationDto( - percent_exposure=0, - variations=[ - VariationDto( - name="control", - value="control", - shard_range=ShardRange(start=0, end=10000), - ) - ], - ) - mock_config_requestor.get_configuration.return_value = ExperimentConfigurationDto( - subject_shards=10000, - enabled=True, - name="recommendation_algo", - overrides=dict(), - allocations={"allocation": allocation}, - ) - client = EppoClient( - config_requestor=mock_config_requestor, assignment_logger=AssignmentLogger() - ) - assert client.get_assignment("user-1", "experiment-key-1") is None - - @patch("eppo_client.assignment_logger.AssignmentLogger") @patch("eppo_client.configuration_requestor.ExperimentConfigurationRequestor") def test_log_assignment(mock_config_requestor, mock_logger): - allocation = AllocationDto( - percent_exposure=1, - variations=[ - VariationDto( - name="control", - value="control", - shard_range=ShardRange(start=0, end=10000), + flag = Flag( + key="flag-key", + enabled=True, + variations={ + "control": Variation( + key="control", value="control", value_type=ValueType.STRING + ) + }, + allocations=[ + Allocation( + key="allocation", + rules=[], + splits=[ + Split( + variation_key="control", + shards=[Shard(salt="salt", ranges=[Range(start=0, end=10000)])], + ) + ], ) ], + total_shards=10_000, ) - mock_config_requestor.get_configuration.return_value = ExperimentConfigurationDto( - allocations={"allocation": allocation}, - rules=[Rule(conditions=[], allocation_key="allocation")], - subject_shards=10000, - enabled=True, - name="recommendation_algo", - overrides=dict(), - ) + + mock_config_requestor.get_configuration.return_value = flag client = EppoClient( config_requestor=mock_config_requestor, assignment_logger=mock_logger ) - assert client.get_assignment("user-1", "experiment-key-1") == "control" + assert client.get_string_assignment("user-1", "flag-key") == "control" assert mock_logger.log_assignment.call_count == 1 @patch("eppo_client.assignment_logger.AssignmentLogger") @patch("eppo_client.configuration_requestor.ExperimentConfigurationRequestor") def test_get_assignment_handles_logging_exception(mock_config_requestor, mock_logger): - allocation = AllocationDto( - percent_exposure=1, - variations=[ - VariationDto( - name="control", - value="control", - shard_range=ShardRange(start=0, end=10000), - ) - ], - ) - mock_config_requestor.get_configuration.return_value = ExperimentConfigurationDto( - subject_shards=10000, - allocations={"allocation": allocation}, + flag = Flag( + key="flag-key", enabled=True, - rules=[Rule(conditions=[], allocation_key="allocation")], - name="recommendation_algo", - overrides=dict(), - ) - mock_logger.log_assignment.side_effect = ValueError("logging error") - client = EppoClient( - config_requestor=mock_config_requestor, assignment_logger=mock_logger - ) - - assert client.get_assignment("user-1", "experiment-key-1") == "control" - - -@patch("eppo_client.configuration_requestor.ExperimentConfigurationRequestor") -def test_assign_subject_with_with_attributes_and_rules(mock_config_requestor): - allocation = AllocationDto( - percent_exposure=1, - variations=[ - VariationDto( - name="control", - value="control", - shard_range=ShardRange(start=0, end=10000), + variations={ + "control": Variation( + key="control", value="control", value_type=ValueType.STRING ) - ], - ) - matches_email_condition = Condition( - operator=OperatorType.MATCHES, value=".*@eppo.com", attribute="email" - ) - text_rule = Rule(conditions=[matches_email_condition], allocation_key="allocation") - mock_config_requestor.get_configuration.return_value = ExperimentConfigurationDto( - subject_shards=10000, - allocations={"allocation": allocation}, - enabled=True, - name="experiment-key-1", - overrides=dict(), - rules=[text_rule], - ) - client = EppoClient( - config_requestor=mock_config_requestor, assignment_logger=AssignmentLogger() - ) - assert client.get_assignment("user-1", "experiment-key-1") is None - assert ( - client.get_assignment( - "user1", "experiment-key-1", {"email": "test@example.com"} - ) - is None - ) - assert ( - client.get_assignment("user1", "experiment-key-1", {"email": "test@eppo.com"}) - == "control" - ) - - -@patch("eppo_client.configuration_requestor.ExperimentConfigurationRequestor") -def test_with_subject_in_overrides(mock_config_requestor): - allocation = AllocationDto( - percent_exposure=1, - variations=[ - VariationDto( - name="control", - value="control", - shard_range=ShardRange(start=0, end=10000), + }, + allocations=[ + Allocation( + key="allocation", + rules=[], + splits=[ + Split( + variation_key="control", + shards=[Shard(salt="salt", ranges=[Range(start=0, end=10000)])], + ) + ], ) ], + total_shards=10_000, ) - mock_config_requestor.get_configuration.return_value = ExperimentConfigurationDto( - subject_shards=10000, - allocations={"allocation": allocation}, - enabled=True, - rules=[Rule(conditions=[], allocation_key="allocation")], - name="recommendation_algo", - overrides={"d6d7705392bc7af633328bea8c4c6904": "override-variation"}, - typed_overrides={"d6d7705392bc7af633328bea8c4c6904": "override-variation"}, - ) - client = EppoClient( - config_requestor=mock_config_requestor, assignment_logger=AssignmentLogger() - ) - assert client.get_assignment("user-1", "experiment-key-1") == "override-variation" + mock_config_requestor.get_configuration.return_value = flag + mock_logger.log_assignment.side_effect = ValueError("logging error") -@patch("eppo_client.configuration_requestor.ExperimentConfigurationRequestor") -def test_with_subject_in_overrides_exp_disabled(mock_config_requestor): - allocation = AllocationDto( - percent_exposure=0, - variations=[ - VariationDto( - name="control", - value="control", - shard_range=ShardRange(start=0, end=10000), - ) - ], - ) - mock_config_requestor.get_configuration.return_value = ExperimentConfigurationDto( - subject_shards=10000, - allocations={"allocation": allocation}, - enabled=False, - rules=[Rule(conditions=[], allocation_key="allocation")], - name="recommendation_algo", - overrides={"d6d7705392bc7af633328bea8c4c6904": "override-variation"}, - typed_overrides={"d6d7705392bc7af633328bea8c4c6904": "override-variation"}, - ) client = EppoClient( - config_requestor=mock_config_requestor, assignment_logger=AssignmentLogger() + config_requestor=mock_config_requestor, assignment_logger=mock_logger ) - assert client.get_assignment("user-1", "experiment-key-1") == "override-variation" + assert client.get_string_assignment("user-1", "flag-key") == "control" @patch("eppo_client.configuration_requestor.ExperimentConfigurationRequestor") @@ -243,13 +151,13 @@ def test_with_null_experiment_config(mock_config_requestor): client = EppoClient( config_requestor=mock_config_requestor, assignment_logger=AssignmentLogger() ) - assert client.get_assignment("user-1", "experiment-key-1") is None + assert client.get_assignment("user-1", "flag-key-1") is None @patch("eppo_client.configuration_requestor.ExperimentConfigurationRequestor") -@patch.object(EppoClient, "get_assignment_variation") -def test_graceful_mode_on(mock_get_assignment_variation, mock_config_requestor): - mock_get_assignment_variation.side_effect = Exception("This is a mock exception!") +@patch.object(EppoClient, "get_assignment_detail") +def test_graceful_mode_on(get_assignment_detail, mock_config_requestor): + get_assignment_detail.side_effect = Exception("This is a mock exception!") client = EppoClient( config_requestor=mock_config_requestor, @@ -258,17 +166,19 @@ def test_graceful_mode_on(mock_get_assignment_variation, mock_config_requestor): ) assert client.get_assignment("user-1", "experiment-key-1") is None - assert client.get_boolean_assignment("user-1", "experiment-key-1") is None - assert client.get_json_string_assignment("user-1", "experiment-key-1") is None - assert client.get_numeric_assignment("user-1", "experiment-key-1") is None - assert client.get_string_assignment("user-1", "experiment-key-1") is None + assert client.get_boolean_assignment("user-1", "experiment-key-1", default=True) + assert client.get_float_assignment("user-1", "experiment-key-1") is None + assert ( + client.get_string_assignment("user-1", "experiment-key-1", default="control") + == "control" + ) assert client.get_parsed_json_assignment("user-1", "experiment-key-1") is None @patch("eppo_client.configuration_requestor.ExperimentConfigurationRequestor") -@patch.object(EppoClient, "get_assignment_variation") -def test_graceful_mode_off(mock_get_assignment_variation, mock_config_requestor): - mock_get_assignment_variation.side_effect = Exception("This is a mock exception!") +@patch.object(EppoClient, "get_assignment_detail") +def test_graceful_mode_off(mock_get_assignment_detail, mock_config_requestor): + mock_get_assignment_detail.side_effect = Exception("This is a mock exception!") client = EppoClient( config_requestor=mock_config_requestor, @@ -279,39 +189,45 @@ def test_graceful_mode_off(mock_get_assignment_variation, mock_config_requestor) with pytest.raises(Exception): client.get_assignment("user-1", "experiment-key-1") client.get_boolean_assignment("user-1", "experiment-key-1") - client.get_json_string_assignment("user-1", "experiment-key-1") client.get_numeric_assignment("user-1", "experiment-key-1") client.get_string_assignment("user-1", "experiment-key-1") client.get_parsed_json_assignment("user-1", "experiment-key-1") +def test_client_has_flags(): + client = get_instance() + assert len(client.get_flag_keys()) > 0, "No flags have been loaded by the client" + + @pytest.mark.parametrize("test_case", test_data) def test_assign_subject_in_sample(test_case): - print("---- Test case for {} Experiment".format(test_case["experiment"])) - assignments = get_assignments(test_case=test_case) - assert assignments == test_case["expectedAssignments"] + client = get_instance() + client.__is_graceful_mode = False + print("flag=") + print(client.get_flag_keys()) + print("---- Test case for {} Experiment".format(test_case["flag"])) + get_assignments(client, test_case=test_case) -def get_assignments(test_case): - client = get_instance() +def get_assignments(client, test_case): get_typed_assignment = { "string": client.get_string_assignment, - "numeric": client.get_numeric_assignment, + "integer": client.get_integer_assignment, + "float": client.get_float_assignment, "boolean": client.get_boolean_assignment, - "json": client.get_json_string_assignment, + "json": client.get_parsed_json_assignment, }[test_case["valueType"]] - return [ - get_typed_assignment(subjectKey, test_case["experiment"]) - for subjectKey in test_case.get("subjects", []) - ] + [ - get_typed_assignment( - subject_key=subject["subjectKey"], - flag_key=test_case["experiment"], - subject_attributes=subject["subjectAttributes"], + print(test_case["flag"]) + for subject in test_case.get("subjects", []): + variation = get_typed_assignment( + subject["subjectKey"], test_case["flag"], subject["subjectAttributes"] + ) + expected_variation = subject["assignment"] + + assert variation == expected_variation, "Failed for subject: {}, got {}".format( + subject["subjectKey"], variation ) - for subject in test_case.get("subjectsWithAttributes", []) - ] @pytest.mark.parametrize("test_case", test_data) @@ -323,3 +239,7 @@ def test_get_numeric_assignment_on_bool_feature_flag_should_return_none(test_cas test_case["valueType"] = "numeric" assignments = get_assignments(test_case=test_case) assert assignments == [None] * len(test_case["expectedAssignments"]) + + +def test_check_type_match(): + assert check_type_match("string", ValueType.STRING) diff --git a/test/eval_test.py b/test/eval_test.py index 574f366..c7082ca 100644 --- a/test/eval_test.py +++ b/test/eval_test.py @@ -1,16 +1,28 @@ import datetime -from eppo_client.models import Flag, Allocation, Range, Variation, Split, Shard +from eppo_client.models import ( + Flag, + Allocation, + Range, + ValueType, + Variation, + Split, + Shard, +) from eppo_client.eval import Evaluator, FlagEvaluation, is_in_shard_range, hash_key from eppo_client.rules import Condition, OperatorType, Rule from eppo_client.sharding import DeterministicSharder, MD5Sharder +VARIATION_A = Variation(key="a", value="A", value_type=ValueType.STRING) +VARIATION_B = Variation(key="b", value="B", value_type=ValueType.STRING) +VARIATION_C = Variation(key="c", value="C", value_type=ValueType.STRING) + def test_disabled_flag_returns_none_result(): flag = Flag( key="disabled_flag", enabled=False, - variations={"a": Variation(key="a", value="A")}, + variations={"a": VARIATION_A}, allocations=[ Allocation( key="default", rules=[], splits=[Split(variation_key="a", shards=[])] @@ -62,8 +74,8 @@ def test_eval_empty_flag(): key="empty", enabled=True, variations={ - "a": Variation(key="a", value="A"), - "b": Variation(key="b", value="B"), + "a": VARIATION_A, + "b": VARIATION_B, }, allocations=[], total_shards=10, @@ -81,13 +93,44 @@ def test_eval_empty_flag(): ) +def test_simple_flag(): + flag = Flag( + key="flag-key", + enabled=True, + variations={ + "control": Variation( + key="control", value="control", value_type=ValueType.STRING + ) + }, + allocations=[ + Allocation( + key="allocation", + rules=[], + splits=[ + Split( + variation_key="control", + shards=[Shard(salt="salt", ranges=[Range(start=0, end=10000)])], + ) + ], + ) + ], + total_shards=10_000, + ) + + evaluator = Evaluator(sharder=MD5Sharder()) + result = evaluator.evaluate_flag(flag, "user-1", {}) + assert result.variation == Variation( + key="control", value="control", value_type=ValueType.STRING + ) + + def test_catch_all_allocation(): flag = Flag( key="flag", enabled=True, variations={ - "a": Variation(key="a", value="A"), - "b": Variation(key="b", value="B"), + "a": VARIATION_A, + "b": VARIATION_B, }, allocations=[ Allocation( @@ -103,7 +146,7 @@ def test_catch_all_allocation(): result = evaluator.evaluate_flag(flag, "subject_key", {}) assert result.flag_key == "flag" assert result.allocation_key == "default" - assert result.variation == Variation(key="a", value="A") + assert result.variation == VARIATION_A assert result.do_log @@ -112,8 +155,8 @@ def test_match_first_allocation_rule(): key="flag", enabled=True, variations={ - "a": Variation(key="a", value="A"), - "b": Variation(key="b", value="B"), + "a": VARIATION_A, + "b": VARIATION_B, }, allocations=[ Allocation( @@ -144,7 +187,7 @@ def test_match_first_allocation_rule(): result = evaluator.evaluate_flag(flag, "subject_key", {"email": "eppo@example.com"}) assert result.flag_key == "flag" assert result.allocation_key == "first" - assert result.variation == Variation(key="b", value="B") + assert result.variation == VARIATION_B def test_do_not_match_first_allocation_rule(): @@ -152,8 +195,8 @@ def test_do_not_match_first_allocation_rule(): key="flag", enabled=True, variations={ - "a": Variation(key="a", value="A"), - "b": Variation(key="b", value="B"), + "a": VARIATION_A, + "b": VARIATION_B, }, allocations=[ Allocation( @@ -184,7 +227,7 @@ def test_do_not_match_first_allocation_rule(): result = evaluator.evaluate_flag(flag, "subject_key", {"email": "eppo@test.com"}) assert result.flag_key == "flag" assert result.allocation_key == "default" - assert result.variation == Variation(key="a", value="A") + assert result.variation == VARIATION_A def test_eval_sharding(): @@ -192,9 +235,9 @@ def test_eval_sharding(): key="flag", enabled=True, variations={ - "a": Variation(key="a", value="A"), - "b": Variation(key="b", value="B"), - "c": Variation(key="c", value="C"), + "a": VARIATION_A, + "b": VARIATION_B, + "c": VARIATION_C, }, allocations=[ Allocation( @@ -243,28 +286,28 @@ def test_eval_sharding(): result = evaluator.evaluate_flag(flag, "alice", {}) assert result.allocation_key == "first" - assert result.variation == Variation(key="a", value="A") + assert result.variation == VARIATION_A result = evaluator.evaluate_flag(flag, "bob", {}) assert result.allocation_key == "first" - assert result.variation == Variation(key="b", value="B") + assert result.variation == VARIATION_B # charlie matches on traffic but not on split and falls through result = evaluator.evaluate_flag(flag, "charlie", {}) assert result.allocation_key == "default" - assert result.variation == Variation(key="c", value="C") + assert result.variation == VARIATION_C # dave does not match traffic result = evaluator.evaluate_flag(flag, "dave", {}) assert result.allocation_key == "default" - assert result.variation == Variation(key="c", value="C") + assert result.variation == VARIATION_C def test_eval_prior_to_alloc(mocker): flag = Flag( key="flag", enabled=True, - variations={"a": Variation(key="a", value="A")}, + variations={"a": VARIATION_A}, allocations=[ Allocation( key="default", @@ -278,20 +321,18 @@ def test_eval_prior_to_alloc(mocker): ) evaluator = Evaluator(sharder=MD5Sharder()) - with mocker.patch( - "eppo_client.eval.utcnow", return_value=datetime.datetime(2023, 1, 1) - ): - result = evaluator.evaluate_flag(flag, "subject_key", {}) - assert result.flag_key == "flag" - assert result.allocation_key == None - assert result.variation == None + mocker.patch("eppo_client.eval.utcnow", return_value=datetime.datetime(2023, 1, 1)) + result = evaluator.evaluate_flag(flag, "subject_key", {}) + assert result.flag_key == "flag" + assert result.allocation_key == None + assert result.variation == None def test_eval_during_alloc(mocker): flag = Flag( key="flag", enabled=True, - variations={"a": Variation(key="a", value="A")}, + variations={"a": VARIATION_A}, allocations=[ Allocation( key="default", @@ -305,20 +346,18 @@ def test_eval_during_alloc(mocker): ) evaluator = Evaluator(sharder=MD5Sharder()) - with mocker.patch( - "eppo_client.eval.utcnow", return_value=datetime.datetime(2024, 1, 5) - ): - result = evaluator.evaluate_flag(flag, "subject_key", {}) - assert result.flag_key == "flag" - assert result.allocation_key == "default" - assert result.variation == Variation(key="a", value="A") + mocker.patch("eppo_client.eval.utcnow", return_value=datetime.datetime(2024, 1, 5)) + result = evaluator.evaluate_flag(flag, "subject_key", {}) + assert result.flag_key == "flag" + assert result.allocation_key == "default" + assert result.variation == VARIATION_A def test_eval_after_alloc(mocker): flag = Flag( key="flag", enabled=True, - variations={"a": Variation(key="a", value="A")}, + variations={"a": VARIATION_A}, allocations=[ Allocation( key="default", @@ -332,13 +371,11 @@ def test_eval_after_alloc(mocker): ) evaluator = Evaluator(sharder=MD5Sharder()) - with mocker.patch( - "eppo_client.eval.utcnow", return_value=datetime.datetime(2024, 2, 5) - ): - result = evaluator.evaluate_flag(flag, "subject_key", {}) - assert result.flag_key == "flag" - assert result.allocation_key == None - assert result.variation == None + mocker.patch("eppo_client.eval.utcnow", return_value=datetime.datetime(2024, 2, 5)) + result = evaluator.evaluate_flag(flag, "subject_key", {}) + assert result.flag_key == "flag" + assert result.allocation_key == None + assert result.variation == None def test_seed(): From 571c2c7ec45bb9d23e5783cb0cc465aea08d537b Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Thu, 14 Mar 2024 15:51:51 -0700 Subject: [PATCH 09/40] update tests --- test/client_test.py | 43 +++++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/test/client_test.py b/test/client_test.py index da0bbdd..e23c19e 100644 --- a/test/client_test.py +++ b/test/client_test.py @@ -202,14 +202,8 @@ def test_client_has_flags(): @pytest.mark.parametrize("test_case", test_data) def test_assign_subject_in_sample(test_case): client = get_instance() - client.__is_graceful_mode = False - print("flag=") - print(client.get_flag_keys()) print("---- Test case for {} Experiment".format(test_case["flag"])) - get_assignments(client, test_case=test_case) - -def get_assignments(client, test_case): get_typed_assignment = { "string": client.get_string_assignment, "integer": client.get_integer_assignment, @@ -218,27 +212,40 @@ def get_assignments(client, test_case): "json": client.get_parsed_json_assignment, }[test_case["valueType"]] + assignments = get_assignments(test_case, get_typed_assignment) + for subject, assigned_variation in assignments: + assert assigned_variation == subject["assignment"] + + +def get_assignments(test_case, get_assignment_fn): + client = get_instance() + client.__is_graceful_mode = False + print(test_case["flag"]) + assignments = [] for subject in test_case.get("subjects", []): - variation = get_typed_assignment( + variation = get_assignment_fn( subject["subjectKey"], test_case["flag"], subject["subjectAttributes"] ) - expected_variation = subject["assignment"] - - assert variation == expected_variation, "Failed for subject: {}, got {}".format( - subject["subjectKey"], variation - ) + assignments.append((subject, variation)) + return assignments @pytest.mark.parametrize("test_case", test_data) def test_get_numeric_assignment_on_bool_feature_flag_should_return_none(test_case): + client = get_instance() if test_case["valueType"] == "boolean": - assignments = get_assignments(test_case=test_case) - assert assignments == test_case["expectedAssignments"] - # Change to get_numeric_assignment and try again - test_case["valueType"] = "numeric" - assignments = get_assignments(test_case=test_case) - assert assignments == [None] * len(test_case["expectedAssignments"]) + assignments = get_assignments( + test_case=test_case, get_assignment_fn=client.get_float_assignment + ) + for _, assigned_variation in assignments: + assert assigned_variation is None + + assignments = get_assignments( + test_case=test_case, get_assignment_fn=client.get_integer_assignment + ) + for _, assigned_variation in assignments: + assert assigned_variation is None def test_check_type_match(): From c698818c1b4d8136ede7b6e591dd8dd10b9c8d79 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Thu, 14 Mar 2024 17:28:51 -0700 Subject: [PATCH 10/40] update tests and json --- eppo_client/client.py | 14 +++++++++----- test/client_test.py | 4 +++- test/rules_test.py | 15 +++++++++++++++ 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/eppo_client/client.py b/eppo_client/client.py index 141ebca..1b4433e 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -1,5 +1,6 @@ import datetime import logging +import json from typing import Any, Dict, Optional, Union from typing_extensions import deprecated from eppo_client.assignment_logger import AssignmentLogger @@ -13,6 +14,7 @@ from eppo_client.validation import validate_not_blank from eppo_client.eval import FlagEvaluation, Evaluator + logger = logging.getLogger(__name__) @@ -116,13 +118,15 @@ def get_parsed_json_assignment( subject_attributes: Optional[Dict[str, Union[str, float, int, bool]]] = None, default=None, ) -> Optional[Dict[Any, Any]]: - return self.get_assignment_variation( + variation_jsons = self.get_assignment_variation( subject_key, flag_key, subject_attributes, ValueType.JSON, default=default, ) + if variation_jsons: + return json.loads(variation_jsons) @deprecated( "get_assignment is deprecated in favor of the typed get__assignment methods" @@ -150,7 +154,7 @@ def get_assignment_variation( result = self.get_assignment_detail( subject_key, flag_key, subject_attributes ) - if not result: + if not result or not result.variation: return default assigned_variation = result.variation if not check_type_match( @@ -200,10 +204,10 @@ def get_assignment_detail( assignment_event = { **result.extra_logging, - "allocation": result.allocation_key, - "experiment": f"{flag_key}-{result.allocation_key}", + "allocation": result.allocation_key if result else None, + "experiment": f"{flag_key}-{result.allocation_key}" if result else None, "featureFlag": flag_key, - "variation": result.variation.key, + "variation": result.variation.key if result and result.variation else None, "subject": subject_key, "timestamp": datetime.datetime.utcnow().isoformat(), "subjectAttributes": subject_attributes, diff --git a/test/client_test.py b/test/client_test.py index e23c19e..20792db 100644 --- a/test/client_test.py +++ b/test/client_test.py @@ -214,7 +214,9 @@ def test_assign_subject_in_sample(test_case): assignments = get_assignments(test_case, get_typed_assignment) for subject, assigned_variation in assignments: - assert assigned_variation == subject["assignment"] + assert ( + assigned_variation == subject["assignment"] + ), f"expected <{subject['assignment']}> for subject {subject['subjectKey']}, found <{assigned_variation}>" def get_assignments(test_case, get_assignment_fn): diff --git a/test/rules_test.py b/test/rules_test.py index 2155526..cc5940b 100644 --- a/test/rules_test.py +++ b/test/rules_test.py @@ -131,6 +131,21 @@ def test_evaluate_condition_not_one_of(): {"name": "charlie"}, ) + # NOT_ONE_OF fails when attribute is not specified + assert not evaluate_condition( + Condition( + operator=OperatorType.NOT_ONE_OF, value=["alice", "bob"], attribute="name" + ), + {}, + ) + + assert not evaluate_condition( + Condition( + operator=OperatorType.NOT_ONE_OF, value=["alice", "bob"], attribute="name" + ), + {"name": None}, + ) + # def test_find_matching_rule_with_empty_rules(): # subject_attributes = {"age": 20, "country": "US"} From 209bfea94ae0f01807d98631f9885b43efba8e56 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Fri, 15 Mar 2024 11:52:59 -0700 Subject: [PATCH 11/40] update tests --- test/rules_test.py | 246 +++++++++++++++------------------------------ 1 file changed, 81 insertions(+), 165 deletions(-) diff --git a/test/rules_test.py b/test/rules_test.py index cc5940b..6c162a1 100644 --- a/test/rules_test.py +++ b/test/rules_test.py @@ -76,20 +76,6 @@ def test_matches_rule_with_true_and_false_condition(): ) -def test_evaluate_condition_matches(): - assert evaluate_condition( - Condition(operator=OperatorType.MATCHES, value=".*", attribute="name"), - {"name": "alice"}, - ) - - -def test_evaluate_condition_matches_false(): - assert not evaluate_condition( - Condition(operator=OperatorType.MATCHES, value="bob", attribute="name"), - {"name": "alice"}, - ) - - def test_evaluate_condition_one_of(): assert evaluate_condition( Condition( @@ -147,154 +133,84 @@ def test_evaluate_condition_not_one_of(): ) -# def test_find_matching_rule_with_empty_rules(): -# subject_attributes = {"age": 20, "country": "US"} -# assert find_matching_rule([], "alice", subject_attributes) is None - - -# def test_find_matching_rule_when_no_rules_match(): -# subject_attributes = {"age": 99, "country": "US", "email": "test@example.com"} -# assert find_matching_rule([text_rule], "alice", subject_attributes) is None - - -# def test_find_matching_rule_on_match(): -# assert find_matching_rule([numeric_rule], "alice", {"age": 99}) == numeric_rule -# assert ( -# find_matching_rule([text_rule], "alice", {"email": "testing@email.com"}) -# == text_rule -# ) - - -# def test_find_matching_rule_if_no_attribute_for_condition(): -# assert find_matching_rule([numeric_rule], "alice", {}) is None - - -# def test_find_matching_rule_if_no_conditions_for_rule(): -# assert ( -# find_matching_rule([rule_with_empty_conditions], "alice", {}) -# == rule_with_empty_conditions -# ) - - -# def test_find_matching_rule_if_numeric_operator_with_string(): -# assert find_matching_rule([numeric_rule], "alice", {"age": "99"}) is None - - -# def test_find_matching_rule_with_numeric_value_and_regex(): -# condition = Condition( -# operator=OperatorType.MATCHES, value="[0-9]+", attribute="age" -# ) -# rule = Rule(conditions=[condition], allocation_key="allocation") -# assert find_matching_rule([rule], "alice", {"age": 99}) == rule - - -# def test_find_matching_rule_with_semver(): -# semver_greater_than_condition = Condition( -# operator=OperatorType.GTE, value="1.0.0", attribute="version" -# ) -# semver_less_than_condition = Condition( -# operator=OperatorType.LTE, value="2.0.0", attribute="version" -# ) -# semver_rule = Rule( -# allocation_key="allocation", -# conditions=[semver_less_than_condition, semver_greater_than_condition], -# ) - -# assert find_matching_rule({"version": "1.1.0"}, [semver_rule]) is semver_rule -# assert find_matching_rule({"version": "2.0.0"}, [semver_rule]) is semver_rule -# assert find_matching_rule({"version": "2.1.0"}, [semver_rule]) is None - - -# def test_one_of_operator_with_boolean(): -# oneOfRule = Rule( -# allocation_key="allocation", -# conditions=[ -# Condition(operator=OperatorType.ONE_OF, value=["True"], attribute="enabled") -# ], -# ) -# notOneOfRule = Rule( -# allocation_key="allocation", -# conditions=[ -# Condition( -# operator=OperatorType.NOT_ONE_OF, value=["True"], attribute="enabled" -# ) -# ], -# ) -# assert find_matching_rule({"enabled": True}, [oneOfRule]) == oneOfRule -# assert find_matching_rule({"enabled": False}, [oneOfRule]) is None -# assert find_matching_rule({"enabled": True}, [notOneOfRule]) is None -# assert find_matching_rule({"enabled": False}, [notOneOfRule]) == notOneOfRule - - -# def test_one_of_operator_case_insensitive(): -# oneOfRule = Rule( -# allocation_key="allocation", -# conditions=[ -# Condition( -# operator=OperatorType.ONE_OF, value=["1Ab", "Ron"], attribute="name" -# ) -# ], -# ) -# assert find_matching_rule({"name": "ron"}, [oneOfRule]) == oneOfRule -# assert find_matching_rule({"name": "1AB"}, [oneOfRule]) == oneOfRule - - -# def test_not_one_of_operator_case_insensitive(): -# notOneOf = Rule( -# allocation_key="allocation", -# conditions=[ -# Condition( -# operator=OperatorType.NOT_ONE_OF, -# value=["bbB", "1.1.ab"], -# attribute="name", -# ) -# ], -# ) -# assert find_matching_rule({"name": "BBB"}, [notOneOf]) is None -# assert find_matching_rule({"name": "1.1.AB"}, [notOneOf]) is None - - -# def test_one_of_operator_with_string(): -# oneOfRule = Rule( -# allocation_key="allocation", -# conditions=[ -# Condition( -# operator=OperatorType.ONE_OF, value=["john", "ron"], attribute="name" -# ) -# ], -# ) -# notOneOfRule = Rule( -# allocation_key="allocation", -# conditions=[ -# Condition(operator=OperatorType.NOT_ONE_OF, value=["ron"], attribute="name") -# ], -# ) -# assert find_matching_rule({"name": "john"}, [oneOfRule]) == oneOfRule -# assert find_matching_rule({"name": "ron"}, [oneOfRule]) == oneOfRule -# assert find_matching_rule({"name": "sam"}, [oneOfRule]) is None -# assert find_matching_rule({"name": "ron"}, [notOneOfRule]) is None -# assert find_matching_rule({"name": "sam"}, [notOneOfRule]) == notOneOfRule - - -# def test_one_of_operator_with_number(): -# oneOfRule = Rule( -# allocation_key="allocation", -# conditions=[ -# Condition( -# operator=OperatorType.ONE_OF, value=["14", "15.11"], attribute="number" -# ) -# ], -# ) -# notOneOfRule = Rule( -# allocation_key="allocation", -# conditions=[ -# Condition( -# operator=OperatorType.NOT_ONE_OF, value=["10"], attribute="number" -# ) -# ], -# ) -# assert find_matching_rule({"number": "14"}, [oneOfRule]) == oneOfRule -# assert find_matching_rule({"number": 15.11}, [oneOfRule]) == oneOfRule -# assert find_matching_rule({"number": "10"}, [oneOfRule]) is None -# assert find_matching_rule({"number": "10"}, [notOneOfRule]) is None -# assert find_matching_rule({"number": 11}, [notOneOfRule]) == notOneOfRule +def test_evaluate_condition_matches(): + assert evaluate_condition( + Condition(operator=OperatorType.MATCHES, value="^test.*", attribute="email"), + {"email": "test@example.com"}, + ) + assert not evaluate_condition( + Condition(operator=OperatorType.MATCHES, value="^test.*", attribute="email"), + {"email": "example@test.com"}, + ) + + +def test_evaluate_condition_gte(): + assert evaluate_condition( + Condition(operator=OperatorType.GTE, value=18, attribute="age"), + {"age": 18}, + ) + assert not evaluate_condition( + Condition(operator=OperatorType.GTE, value=18, attribute="age"), + {"age": 17}, + ) + + +def test_evaluate_condition_gt(): + assert evaluate_condition( + Condition(operator=OperatorType.GT, value=18, attribute="age"), + {"age": 19}, + ) + assert not evaluate_condition( + Condition(operator=OperatorType.GT, value=18, attribute="age"), + {"age": 18}, + ) + + +def test_evaluate_condition_lte(): + assert evaluate_condition( + Condition(operator=OperatorType.LTE, value=18, attribute="age"), + {"age": 18}, + ) + assert not evaluate_condition( + Condition(operator=OperatorType.LTE, value=18, attribute="age"), + {"age": 19}, + ) + + +def test_evaluate_condition_lt(): + assert evaluate_condition( + Condition(operator=OperatorType.LT, value=18, attribute="age"), + {"age": 17}, + ) + assert not evaluate_condition( + Condition(operator=OperatorType.LT, value=18, attribute="age"), + {"age": 18}, + ) + + +def test_evaluate_condition_semver(): + assert evaluate_condition( + Condition(operator=OperatorType.GTE, value="1.0.0", attribute="version"), + {"version": "1.0.1"}, + ) + assert not evaluate_condition( + Condition(operator=OperatorType.GTE, value="1.0.0", attribute="version"), + {"version": "0.9.9"}, + ) + + +def test_one_of_operator_with_number(): + one_of_condition = Condition( + operator=OperatorType.ONE_OF, value=["14", "15.11"], attribute="number" + ) + not_one_of_condition = Condition( + operator=OperatorType.NOT_ONE_OF, value=["10"], attribute="number" + ) + assert evaluate_condition(one_of_condition, {"number": "14"}) + assert evaluate_condition(one_of_condition, {"number": 14}) + assert not evaluate_condition(one_of_condition, {"number": 10}) + assert not evaluate_condition(one_of_condition, {"number": "10"}) + assert not evaluate_condition(not_one_of_condition, {"number": "10"}) + assert not evaluate_condition(not_one_of_condition, {"number": 10}) + assert evaluate_condition(not_one_of_condition, {"number": "11"}) + assert evaluate_condition(not_one_of_condition, {"number": 11}) From bdcbb6154c20b01dd765740c1a80b9629240f24c Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Fri, 15 Mar 2024 12:58:40 -0700 Subject: [PATCH 12/40] move value type to flag --- eppo_client/client.py | 33 ++++++++++++++++++--------------- eppo_client/models.py | 2 +- test/client_test.py | 16 +++++----------- 3 files changed, 24 insertions(+), 27 deletions(-) diff --git a/eppo_client/client.py b/eppo_client/client.py index 1b4433e..84d2a6a 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -147,23 +147,16 @@ def get_assignment_variation( subject_key: str, flag_key: str, subject_attributes: Optional[Dict[str, Union[str, float, int, bool]]] = None, - expected_variation_type: Optional[ValueType] = None, + expected_value_type: Optional[ValueType] = None, default=None, ): try: result = self.get_assignment_detail( - subject_key, flag_key, subject_attributes + subject_key, flag_key, subject_attributes, expected_value_type ) if not result or not result.variation: return default - assigned_variation = result.variation - if not check_type_match( - assigned_variation.value_type, expected_variation_type - ): - raise TypeError( - "Variation value does not have the correct type. Found: {assigned_variation.value_type} != {expected_variation_type}" - ) - return assigned_variation.value + return result.variation.value except ValueError as e: # allow ValueError to bubble up as it is a validation error raise e @@ -178,6 +171,7 @@ def get_assignment_detail( subject_key: str, flag_key: str, subject_attributes: Optional[Dict[str, Union[str, float, int, bool]]] = None, + expected_value_type: Optional[ValueType] = None, ) -> Optional[FlagEvaluation]: """Maps a subject to a variation for a given experiment Returns None if the subject is not part of the experiment sample. @@ -194,9 +188,18 @@ def get_assignment_detail( flag = self.__config_requestor.get_configuration(flag_key) - if flag is None or not flag.enabled: + if flag is None: + logger.info("[Eppo SDK] No assigned variation. Flag not found: " + flag_key) + return None + + if not check_type_match(expected_value_type, flag.value_type): + raise TypeError( + "Variation value does not have the correct type. Found: {flag.value_type} != {expected_value_type}" + ) + + if not flag.enabled: logger.info( - "[Eppo SDK] No assigned variation. No active flag for key: " + flag_key + "[Eppo SDK] No assigned variation. Flag is disabled: " + flag_key ) return None @@ -229,9 +232,9 @@ def _shutdown(self): self.__poller.stop() -def check_type_match(value_type, expected_type): +def check_type_match(expected_type, actual_type): return ( expected_type is None - or value_type == expected_type - or value_type == expected_type.value + or actual_type == expected_type + or actual_type == expected_type.value ) diff --git a/eppo_client/models.py b/eppo_client/models.py index abdb704..26a9937 100644 --- a/eppo_client/models.py +++ b/eppo_client/models.py @@ -18,7 +18,6 @@ class ValueType(Enum): class Variation(SdkBaseModel): key: str value: Union[str, int, float, bool] - value_type: ValueType class Range(SdkBaseModel): @@ -49,6 +48,7 @@ class Allocation(SdkBaseModel): class Flag(SdkBaseModel): key: str enabled: bool + value_type: ValueType variations: Dict[str, Variation] allocations: List[Allocation] total_shards: int = 10_000 diff --git a/test/client_test.py b/test/client_test.py index 20792db..cd8fe92 100644 --- a/test/client_test.py +++ b/test/client_test.py @@ -82,11 +82,8 @@ def test_log_assignment(mock_config_requestor, mock_logger): flag = Flag( key="flag-key", enabled=True, - variations={ - "control": Variation( - key="control", value="control", value_type=ValueType.STRING - ) - }, + valueType=ValueType.STRING, + variations={"control": Variation(key="control", value="control")}, allocations=[ Allocation( key="allocation", @@ -116,11 +113,8 @@ def test_get_assignment_handles_logging_exception(mock_config_requestor, mock_lo flag = Flag( key="flag-key", enabled=True, - variations={ - "control": Variation( - key="control", value="control", value_type=ValueType.STRING - ) - }, + value_type=ValueType.STRING, + variations={"control": Variation(key="control", value="control")}, allocations=[ Allocation( key="allocation", @@ -251,4 +245,4 @@ def test_get_numeric_assignment_on_bool_feature_flag_should_return_none(test_cas def test_check_type_match(): - assert check_type_match("string", ValueType.STRING) + assert check_type_match(ValueType.STRING, "string") From 7cae3d551ede923294c5a4cc077b543cb70077cb Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Fri, 15 Mar 2024 13:56:04 -0700 Subject: [PATCH 13/40] fixes --- eppo_client/client.py | 22 +++++++++++----------- eppo_client/models.py | 4 ++-- test/client_test.py | 12 ++++++------ test/configuration_store_test.py | 8 +++++++- test/eval_test.py | 22 ++++++++++++++++------ 5 files changed, 42 insertions(+), 26 deletions(-) diff --git a/eppo_client/client.py b/eppo_client/client.py index 84d2a6a..ad3c8d9 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -8,7 +8,7 @@ ExperimentConfigurationRequestor, ) from eppo_client.constants import POLL_INTERVAL_MILLIS, POLL_JITTER_MILLIS -from eppo_client.models import ValueType +from eppo_client.models import VariationType from eppo_client.poller import Poller from eppo_client.sharding import MD5Sharder from eppo_client.validation import validate_not_blank @@ -47,7 +47,7 @@ def get_string_assignment( subject_key, flag_key, subject_attributes, - ValueType.STRING, + VariationType.STRING, default=default, ) @@ -62,7 +62,7 @@ def get_integer_assignment( subject_key, flag_key, subject_attributes, - ValueType.INTEGER, + VariationType.INTEGER, default=default, ) @@ -77,7 +77,7 @@ def get_float_assignment( subject_key, flag_key, subject_attributes, - ValueType.FLOAT, + VariationType.FLOAT, default=default, ) @@ -107,7 +107,7 @@ def get_boolean_assignment( subject_key, flag_key, subject_attributes, - ValueType.BOOLEAN, + VariationType.BOOLEAN, default=default, ) @@ -122,7 +122,7 @@ def get_parsed_json_assignment( subject_key, flag_key, subject_attributes, - ValueType.JSON, + VariationType.JSON, default=default, ) if variation_jsons: @@ -147,12 +147,12 @@ def get_assignment_variation( subject_key: str, flag_key: str, subject_attributes: Optional[Dict[str, Union[str, float, int, bool]]] = None, - expected_value_type: Optional[ValueType] = None, + expected_variation_type: Optional[VariationType] = None, default=None, ): try: result = self.get_assignment_detail( - subject_key, flag_key, subject_attributes, expected_value_type + subject_key, flag_key, subject_attributes, expected_variation_type ) if not result or not result.variation: return default @@ -171,7 +171,7 @@ def get_assignment_detail( subject_key: str, flag_key: str, subject_attributes: Optional[Dict[str, Union[str, float, int, bool]]] = None, - expected_value_type: Optional[ValueType] = None, + expected_variation_type: Optional[VariationType] = None, ) -> Optional[FlagEvaluation]: """Maps a subject to a variation for a given experiment Returns None if the subject is not part of the experiment sample. @@ -192,9 +192,9 @@ def get_assignment_detail( logger.info("[Eppo SDK] No assigned variation. Flag not found: " + flag_key) return None - if not check_type_match(expected_value_type, flag.value_type): + if not check_type_match(expected_variation_type, flag.variation_type): raise TypeError( - "Variation value does not have the correct type. Found: {flag.value_type} != {expected_value_type}" + f"Variation value does not have the correct type. Found: {flag.variation_type} != {expected_variation_type}" ) if not flag.enabled: diff --git a/eppo_client/models.py b/eppo_client/models.py index 26a9937..e40c678 100644 --- a/eppo_client/models.py +++ b/eppo_client/models.py @@ -7,7 +7,7 @@ from eppo_client.rules import Rule -class ValueType(Enum): +class VariationType(Enum): STRING = "string" INTEGER = "integer" FLOAT = "float" @@ -48,7 +48,7 @@ class Allocation(SdkBaseModel): class Flag(SdkBaseModel): key: str enabled: bool - value_type: ValueType + variation_type: VariationType variations: Dict[str, Variation] allocations: List[Allocation] total_shards: int = 10_000 diff --git a/test/client_test.py b/test/client_test.py index cd8fe92..06e83ed 100644 --- a/test/client_test.py +++ b/test/client_test.py @@ -13,7 +13,7 @@ Range, Shard, Split, - ValueType, + VariationType, Variation, ) from eppo_client.rules import Condition, OperatorType, Rule @@ -82,7 +82,7 @@ def test_log_assignment(mock_config_requestor, mock_logger): flag = Flag( key="flag-key", enabled=True, - valueType=ValueType.STRING, + variation_type=VariationType.STRING, variations={"control": Variation(key="control", value="control")}, allocations=[ Allocation( @@ -113,7 +113,7 @@ def test_get_assignment_handles_logging_exception(mock_config_requestor, mock_lo flag = Flag( key="flag-key", enabled=True, - value_type=ValueType.STRING, + variation_type=VariationType.STRING, variations={"control": Variation(key="control", value="control")}, allocations=[ Allocation( @@ -204,7 +204,7 @@ def test_assign_subject_in_sample(test_case): "float": client.get_float_assignment, "boolean": client.get_boolean_assignment, "json": client.get_parsed_json_assignment, - }[test_case["valueType"]] + }[test_case["variationType"]] assignments = get_assignments(test_case, get_typed_assignment) for subject, assigned_variation in assignments: @@ -230,7 +230,7 @@ def get_assignments(test_case, get_assignment_fn): @pytest.mark.parametrize("test_case", test_data) def test_get_numeric_assignment_on_bool_feature_flag_should_return_none(test_case): client = get_instance() - if test_case["valueType"] == "boolean": + if test_case["variationType"] == "boolean": assignments = get_assignments( test_case=test_case, get_assignment_fn=client.get_float_assignment ) @@ -245,4 +245,4 @@ def test_get_numeric_assignment_on_bool_feature_flag_should_return_none(test_cas def test_check_type_match(): - assert check_type_match(ValueType.STRING, "string") + assert check_type_match(VariationType.STRING, "string") diff --git a/test/configuration_store_test.py b/test/configuration_store_test.py index 5b8ca30..0cc0d4a 100644 --- a/test/configuration_store_test.py +++ b/test/configuration_store_test.py @@ -1,12 +1,18 @@ from eppo_client.models import Flag from eppo_client.configuration_store import ConfigurationStore +from eppo_client.models import VariationType TEST_MAX_SIZE = 10 store: ConfigurationStore[str] = ConfigurationStore(max_size=TEST_MAX_SIZE) mock_flag = Flag( - key="mock_flag", enabled=True, variations={}, allocations=[], total_shards=10000 + key="mock_flag", + variation_type=VariationType.STRING, + enabled=True, + variations={}, + allocations=[], + total_shards=10000, ) diff --git a/test/eval_test.py b/test/eval_test.py index c7082ca..e2ae131 100644 --- a/test/eval_test.py +++ b/test/eval_test.py @@ -4,7 +4,7 @@ Flag, Allocation, Range, - ValueType, + VariationType, Variation, Split, Shard, @@ -13,15 +13,16 @@ from eppo_client.rules import Condition, OperatorType, Rule from eppo_client.sharding import DeterministicSharder, MD5Sharder -VARIATION_A = Variation(key="a", value="A", value_type=ValueType.STRING) -VARIATION_B = Variation(key="b", value="B", value_type=ValueType.STRING) -VARIATION_C = Variation(key="c", value="C", value_type=ValueType.STRING) +VARIATION_A = Variation(key="a", value="A") +VARIATION_B = Variation(key="b", value="B") +VARIATION_C = Variation(key="c", value="C") def test_disabled_flag_returns_none_result(): flag = Flag( key="disabled_flag", enabled=False, + variation_type=VariationType.STRING, variations={"a": VARIATION_A}, allocations=[ Allocation( @@ -73,6 +74,7 @@ def test_eval_empty_flag(): empty_flag = Flag( key="empty", enabled=True, + variation_type=VariationType.STRING, variations={ "a": VARIATION_A, "b": VARIATION_B, @@ -97,9 +99,10 @@ def test_simple_flag(): flag = Flag( key="flag-key", enabled=True, + variation_type=VariationType.STRING, variations={ "control": Variation( - key="control", value="control", value_type=ValueType.STRING + key="control", value="control", value_type=VariationType.STRING ) }, allocations=[ @@ -120,7 +123,7 @@ def test_simple_flag(): evaluator = Evaluator(sharder=MD5Sharder()) result = evaluator.evaluate_flag(flag, "user-1", {}) assert result.variation == Variation( - key="control", value="control", value_type=ValueType.STRING + key="control", value="control", value_type=VariationType.STRING ) @@ -128,6 +131,7 @@ def test_catch_all_allocation(): flag = Flag( key="flag", enabled=True, + variation_type=VariationType.STRING, variations={ "a": VARIATION_A, "b": VARIATION_B, @@ -154,6 +158,7 @@ def test_match_first_allocation_rule(): flag = Flag( key="flag", enabled=True, + variation_type=VariationType.STRING, variations={ "a": VARIATION_A, "b": VARIATION_B, @@ -194,6 +199,7 @@ def test_do_not_match_first_allocation_rule(): flag = Flag( key="flag", enabled=True, + variation_type=VariationType.STRING, variations={ "a": VARIATION_A, "b": VARIATION_B, @@ -234,6 +240,7 @@ def test_eval_sharding(): flag = Flag( key="flag", enabled=True, + variation_type=VariationType.STRING, variations={ "a": VARIATION_A, "b": VARIATION_B, @@ -307,6 +314,7 @@ def test_eval_prior_to_alloc(mocker): flag = Flag( key="flag", enabled=True, + variation_type=VariationType.STRING, variations={"a": VARIATION_A}, allocations=[ Allocation( @@ -332,6 +340,7 @@ def test_eval_during_alloc(mocker): flag = Flag( key="flag", enabled=True, + variation_type=VariationType.STRING, variations={"a": VARIATION_A}, allocations=[ Allocation( @@ -357,6 +366,7 @@ def test_eval_after_alloc(mocker): flag = Flag( key="flag", enabled=True, + variation_type=VariationType.STRING, variations={"a": VARIATION_A}, allocations=[ Allocation( From a95e0d88a4f9994331de8c25d38f00ef61dbc581 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Fri, 15 Mar 2024 15:03:33 -0700 Subject: [PATCH 14/40] update makefile to point to ufc tests --- Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Makefile b/Makefile index c6e63f5..166f3eb 100644 --- a/Makefile +++ b/Makefile @@ -34,8 +34,7 @@ test-data: rm -rf $(testDataDir) mkdir -p $(tempDir) git clone -b ${branchName} --depth 1 --single-branch ${githubRepoLink} ${gitDataDir} - cp ${gitDataDir}rac-experiments-v3.json ${testDataDir} - cp -r ${gitDataDir}assignment-v2 ${testDataDir} + cp -r ${gitDataDir}ufc ${testDataDir} rm -rf ${tempDir} .PHONY: test From 4252e78f6835fdcaada6268ada03ac55112881a2 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Fri, 15 Mar 2024 15:05:52 -0700 Subject: [PATCH 15/40] update version --- eppo_client/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eppo_client/__init__.py b/eppo_client/__init__.py index 6fbe3f5..4231fff 100644 --- a/eppo_client/__init__.py +++ b/eppo_client/__init__.py @@ -10,7 +10,7 @@ from eppo_client.models import Flag from eppo_client.read_write_lock import ReadWriteLock -__version__ = "1.3.1" +__version__ = "3.0.0" __client: Optional[EppoClient] = None __lock = ReadWriteLock() From 9f8f7f73bad4a28f633118c0b96e8b95ab1235c3 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Fri, 15 Mar 2024 15:28:38 -0700 Subject: [PATCH 16/40] flake8 + mypy fixes --- eppo_client/client.py | 12 +++++++----- eppo_client/eval.py | 4 ++-- eppo_client/rules.py | 2 +- eppo_client/sharding.py | 2 +- test/client_test.py | 1 - test/eval_test.py | 12 ++++++------ test/rules_test.py | 5 ++--- 7 files changed, 19 insertions(+), 19 deletions(-) diff --git a/eppo_client/client.py b/eppo_client/client.py index ad3c8d9..a6f319f 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -125,8 +125,9 @@ def get_parsed_json_assignment( VariationType.JSON, default=default, ) - if variation_jsons: - return json.loads(variation_jsons) + if variation_jsons is None: + return None + return json.loads(variation_jsons) @deprecated( "get_assignment is deprecated in favor of the typed get__assignment methods" @@ -194,7 +195,8 @@ def get_assignment_detail( if not check_type_match(expected_variation_type, flag.variation_type): raise TypeError( - f"Variation value does not have the correct type. Found: {flag.variation_type} != {expected_variation_type}" + f"Variation value does not have the correct type." + f" Found: {flag.variation_type} != {expected_variation_type}" ) if not flag.enabled: @@ -206,7 +208,7 @@ def get_assignment_detail( result = self.__evaluator.evaluate_flag(flag, subject_key, subject_attributes) assignment_event = { - **result.extra_logging, + **(result.extra_logging if result else {}), "allocation": result.allocation_key if result else None, "experiment": f"{flag_key}-{result.allocation_key}" if result else None, "featureFlag": flag_key, @@ -216,7 +218,7 @@ def get_assignment_detail( "subjectAttributes": subject_attributes, } try: - if result.do_log: + if result and result.do_log: self.__assignment_logger.log_assignment(assignment_event) except Exception as e: logger.error("[Eppo SDK] Error logging assignment event: " + str(e)) diff --git a/eppo_client/eval.py b/eppo_client/eval.py index eef9d84..51a7c07 100644 --- a/eppo_client/eval.py +++ b/eppo_client/eval.py @@ -11,8 +11,8 @@ class FlagEvaluation: flag_key: str subject_key: str subject_attributes: Dict[str, Union[str, float, int, bool]] - allocation_key: str - variation: Variation + allocation_key: Optional[str] + variation: Optional[Variation] extra_logging: Dict[str, str] do_log: bool diff --git a/eppo_client/rules.py b/eppo_client/rules.py index 41dae8e..a7e2fa6 100644 --- a/eppo_client/rules.py +++ b/eppo_client/rules.py @@ -55,7 +55,7 @@ def evaluate_condition( # Numeric operator: value could be numeric or semver. if isinstance(subject_value, numbers.Number): return evaluate_numeric_condition(subject_value, condition) - elif is_valid_semver(subject_value): + elif isinstance(subject_value, str) and is_valid_semver(subject_value): return compare_semver( subject_value, condition.value, condition.operator ) diff --git a/eppo_client/sharding.py b/eppo_client/sharding.py index 7b9fb36..8271a4e 100644 --- a/eppo_client/sharding.py +++ b/eppo_client/sharding.py @@ -23,7 +23,7 @@ class DeterministicSharder(Sharder): to simplify writing tests """ - def __init__(self, lookup: Dict[str, str]): + def __init__(self, lookup: Dict[str, int]): self.lookup = lookup def get_shard(self, input: str, total_shards: int) -> int: diff --git a/test/client_test.py b/test/client_test.py index 06e83ed..7e7f7f5 100644 --- a/test/client_test.py +++ b/test/client_test.py @@ -16,7 +16,6 @@ VariationType, Variation, ) -from eppo_client.rules import Condition, OperatorType, Rule from eppo_client import init, get_instance import logging diff --git a/test/eval_test.py b/test/eval_test.py index e2ae131..62a367d 100644 --- a/test/eval_test.py +++ b/test/eval_test.py @@ -35,8 +35,8 @@ def test_disabled_flag_returns_none_result(): evaluator = Evaluator(sharder=MD5Sharder()) result = evaluator.evaluate_flag(flag, "subject_key", {}) assert result.flag_key == "disabled_flag" - assert result.allocation_key == None - assert result.variation == None + assert result.allocation_key is None + assert result.variation is None assert not result.do_log @@ -332,8 +332,8 @@ def test_eval_prior_to_alloc(mocker): mocker.patch("eppo_client.eval.utcnow", return_value=datetime.datetime(2023, 1, 1)) result = evaluator.evaluate_flag(flag, "subject_key", {}) assert result.flag_key == "flag" - assert result.allocation_key == None - assert result.variation == None + assert result.allocation_key is None + assert result.variation is None def test_eval_during_alloc(mocker): @@ -384,8 +384,8 @@ def test_eval_after_alloc(mocker): mocker.patch("eppo_client.eval.utcnow", return_value=datetime.datetime(2024, 2, 5)) result = evaluator.evaluate_flag(flag, "subject_key", {}) assert result.flag_key == "flag" - assert result.allocation_key == None - assert result.variation == None + assert result.allocation_key is None + assert result.variation is None def test_seed(): diff --git a/test/rules_test.py b/test/rules_test.py index 6c162a1..1d82e66 100644 --- a/test/rules_test.py +++ b/test/rules_test.py @@ -9,16 +9,15 @@ greater_than_condition = Condition(operator=OperatorType.GT, value=10, attribute="age") less_than_condition = Condition(operator=OperatorType.LT, value=100, attribute="age") numeric_rule = Rule( - allocation_key="allocation", conditions=[less_than_condition, greater_than_condition], ) matches_email_condition = Condition( operator=OperatorType.MATCHES, value=".*@email.com", attribute="email" ) -text_rule = Rule(allocation_key="allocation", conditions=[matches_email_condition]) +text_rule = Rule(conditions=[matches_email_condition]) -rule_with_empty_conditions = Rule(allocation_key="allocation", conditions=[]) +rule_with_empty_conditions = Rule(conditions=[]) def test_matches_rule_with_empty_rule(): From 6d380563d172b32957977fd313f209c56f783ca1 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Fri, 15 Mar 2024 15:47:45 -0700 Subject: [PATCH 17/40] inject id into subject attributes --- eppo_client/eval.py | 3 ++- test/eval_test.py | 46 ++++++++++++++++++++++++++++++++++++++------- 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/eppo_client/eval.py b/eppo_client/eval.py index 51a7c07..b54f072 100644 --- a/eppo_client/eval.py +++ b/eppo_client/eval.py @@ -42,7 +42,8 @@ def evaluate_flag( # So we look for (rule 1) OR (rule 2) OR (rule 3) etc. # If there are no rules, then we always match if not allocation.rules or any( - matches_rule(rule, subject_attributes) for rule in allocation.rules + matches_rule(rule, {"id": subject_key, **subject_attributes}) + for rule in allocation.rules ): for split in allocation.splits: # Split needs to match all shards diff --git a/test/eval_test.py b/test/eval_test.py index 62a367d..77cd9bb 100644 --- a/test/eval_test.py +++ b/test/eval_test.py @@ -100,11 +100,7 @@ def test_simple_flag(): key="flag-key", enabled=True, variation_type=VariationType.STRING, - variations={ - "control": Variation( - key="control", value="control", value_type=VariationType.STRING - ) - }, + variations={"control": Variation(key="control", value="control")}, allocations=[ Allocation( key="allocation", @@ -122,10 +118,46 @@ def test_simple_flag(): evaluator = Evaluator(sharder=MD5Sharder()) result = evaluator.evaluate_flag(flag, "user-1", {}) - assert result.variation == Variation( - key="control", value="control", value_type=VariationType.STRING + assert result.variation == Variation(key="control", value="control") + + +def test_flag_target_on_id(): + flag = Flag( + key="flag-key", + enabled=True, + variation_type=VariationType.STRING, + variations={"control": Variation(key="control", value="control")}, + allocations=[ + Allocation( + key="allocation", + rules=[ + Rule( + conditions=[ + Condition( + operator=OperatorType.ONE_OF, + attribute="id", + value=["user-1", "user-2"], + ) + ] + ) + ], + splits=[ + Split( + variation_key="control", + shards=[], + ) + ], + ) + ], + total_shards=10_000, ) + evaluator = Evaluator(sharder=MD5Sharder()) + result = evaluator.evaluate_flag(flag, "user-1", {}) + assert result.variation == Variation(key="control", value="control") + result = evaluator.evaluate_flag(flag, "user-3", {}) + assert result.variation is None + def test_catch_all_allocation(): flag = Flag( From 17f09b57e139afc04402d45e1bf79762e705405a Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Mon, 18 Mar 2024 17:20:42 -0700 Subject: [PATCH 18/40] Address Giorgio's comments --- eppo_client/client.py | 29 +++++++++++++++++++---------- eppo_client/models.py | 7 +++++-- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/eppo_client/client.py b/eppo_client/client.py index a6f319f..ac2e660 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -17,6 +17,9 @@ logger = logging.getLogger(__name__) +AttributeValue = Union[str, float, int, bool] +SubjectAttributes = Dict[str, AttributeValue] + class EppoClient: def __init__( @@ -40,7 +43,7 @@ def get_string_assignment( self, subject_key: str, flag_key: str, - subject_attributes: Optional[Dict[str, Union[str, float, int, bool]]] = None, + subject_attributes: Optional[SubjectAttributes] = None, default=None, ) -> Optional[str]: return self.get_assignment_variation( @@ -55,7 +58,7 @@ def get_integer_assignment( self, subject_key: str, flag_key: str, - subject_attributes: Optional[Dict[str, Union[str, float, int, bool]]] = None, + subject_attributes: Optional[SubjectAttributes] = None, default=None, ) -> Optional[float]: return self.get_assignment_variation( @@ -70,7 +73,7 @@ def get_float_assignment( self, subject_key: str, flag_key: str, - subject_attributes: Optional[Dict[str, Union[str, float, int, bool]]] = None, + subject_attributes: Optional[SubjectAttributes] = None, default=None, ) -> Optional[float]: return self.get_assignment_variation( @@ -86,7 +89,7 @@ def get_numeric_assignment( self, subject_key: str, flag_key: str, - subject_attributes: Optional[Dict[str, Union[str, float, int, bool]]] = None, + subject_attributes: Optional[SubjectAttributes] = None, default=None, ) -> Optional[float]: return self.get_float_assignment( @@ -100,7 +103,7 @@ def get_boolean_assignment( self, subject_key: str, flag_key: str, - subject_attributes: Optional[Dict[str, Union[str, float, int, bool]]] = None, + subject_attributes: Optional[SubjectAttributes] = None, default=None, ) -> Optional[bool]: return self.get_assignment_variation( @@ -115,7 +118,7 @@ def get_parsed_json_assignment( self, subject_key: str, flag_key: str, - subject_attributes: Optional[Dict[str, Union[str, float, int, bool]]] = None, + subject_attributes: Optional[SubjectAttributes] = None, default=None, ) -> Optional[Dict[Any, Any]]: variation_jsons = self.get_assignment_variation( @@ -136,7 +139,7 @@ def get_assignment( self, subject_key: str, flag_key: str, - subject_attributes: Optional[Dict[str, Union[str, float, int, bool]]] = None, + subject_attributes: Optional[SubjectAttributes] = None, default=None, ) -> Optional[str]: return self.get_assignment_variation( @@ -147,7 +150,7 @@ def get_assignment_variation( self, subject_key: str, flag_key: str, - subject_attributes: Optional[Dict[str, Union[str, float, int, bool]]] = None, + subject_attributes: Optional[SubjectAttributes] = None, expected_variation_type: Optional[VariationType] = None, default=None, ): @@ -171,7 +174,7 @@ def get_assignment_detail( self, subject_key: str, flag_key: str, - subject_attributes: Optional[Dict[str, Union[str, float, int, bool]]] = None, + subject_attributes: Optional[SubjectAttributes] = None, expected_variation_type: Optional[VariationType] = None, ) -> Optional[FlagEvaluation]: """Maps a subject to a variation for a given experiment @@ -184,7 +187,7 @@ def get_assignment_detail( """ validate_not_blank("subject_key", subject_key) validate_not_blank("flag_key", flag_key) - if not subject_attributes: + if subject_attributes is None: subject_attributes = {} flag = self.__config_requestor.get_configuration(flag_key) @@ -225,6 +228,12 @@ def get_assignment_detail( return result def get_flag_keys(self): + """ + Returns a list of all flag keys that have been initialized. + This can be useful to debug the initialization process. + + Note that it is generally not a good idea to pre-load all flag configurations. + """ return self.__config_requestor.get_flag_keys() def _shutdown(self): diff --git a/eppo_client/models.py b/eppo_client/models.py index e40c678..64fd074 100644 --- a/eppo_client/models.py +++ b/eppo_client/models.py @@ -15,9 +15,12 @@ class VariationType(Enum): JSON = "json" +ValueType = Union[str, int, float, bool] + + class Variation(SdkBaseModel): key: str - value: Union[str, int, float, bool] + value: ValueType class Range(SdkBaseModel): @@ -38,7 +41,7 @@ class Split(SdkBaseModel): class Allocation(SdkBaseModel): key: str - rules: List[Rule] + rules: List[Rule] = [] start_at: Optional[datetime] = None end_at: Optional[datetime] = None splits: List[Split] From a51777974d5209c57e84030f6b54e6445a9d6c04 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Mon, 18 Mar 2024 17:26:05 -0700 Subject: [PATCH 19/40] fix check_type_match --- eppo_client/client.py | 10 ++++------ test/client_test.py | 3 ++- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/eppo_client/client.py b/eppo_client/client.py index ac2e660..cb864e1 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -243,9 +243,7 @@ def _shutdown(self): self.__poller.stop() -def check_type_match(expected_type, actual_type): - return ( - expected_type is None - or actual_type == expected_type - or actual_type == expected_type.value - ) +def check_type_match( + expected_type: Optional[VariationType], actual_type: VariationType +): + return expected_type is None or actual_type == expected_type diff --git a/test/client_test.py b/test/client_test.py index 7e7f7f5..2eadf03 100644 --- a/test/client_test.py +++ b/test/client_test.py @@ -244,4 +244,5 @@ def test_get_numeric_assignment_on_bool_feature_flag_should_return_none(test_cas def test_check_type_match(): - assert check_type_match(VariationType.STRING, "string") + assert check_type_match(VariationType.STRING, VariationType.STRING) + assert check_type_match(None, VariationType.STRING) From b231589e6d820cf3d4a91f4b9b980567cf4f7307 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Wed, 20 Mar 2024 16:28:59 -0700 Subject: [PATCH 20/40] eval always returns a FlagEvaluation --- eppo_client/eval.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eppo_client/eval.py b/eppo_client/eval.py index b54f072..96db7b9 100644 --- a/eppo_client/eval.py +++ b/eppo_client/eval.py @@ -26,7 +26,7 @@ def evaluate_flag( flag: Flag, subject_key: str, subject_attributes: Dict[str, Union[str, float, int, bool]], - ) -> Optional[FlagEvaluation]: + ) -> FlagEvaluation: if not flag.enabled: return none_result(flag.key, subject_key, subject_attributes) From 502f8f33cb4976acc024e4240cdfe18668396f79 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Wed, 20 Mar 2024 16:29:55 -0700 Subject: [PATCH 21/40] add type signatures --- eppo_client/eval.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/eppo_client/eval.py b/eppo_client/eval.py index 96db7b9..d860713 100644 --- a/eppo_client/eval.py +++ b/eppo_client/eval.py @@ -73,11 +73,11 @@ def is_in_shard_range(shard: int, range: Range) -> bool: return range.start <= shard < range.end -def hash_key(salt, subject_key): +def hash_key(salt: str, subject_key: str) -> str: return f"{salt}-{subject_key}" -def none_result(flag_key, subject_key, subject_attributes): +def none_result(flag_key, subject_key, subject_attributes) -> FlagEvaluation: return FlagEvaluation( flag_key=flag_key, subject_key=subject_key, @@ -89,5 +89,5 @@ def none_result(flag_key, subject_key, subject_attributes): ) -def utcnow(): +def utcnow() -> datetime.datetime: return datetime.datetime.utcnow() From 2ba14c11f432789b65adfe525695733a12e67aa0 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Thu, 21 Mar 2024 09:39:29 -0700 Subject: [PATCH 22/40] more typing --- eppo_client/client.py | 6 ++---- eppo_client/eval.py | 12 ++++++++---- eppo_client/models.py | 6 ++---- eppo_client/rules.py | 11 +++++------ eppo_client/types.py | 6 ++++++ 5 files changed, 23 insertions(+), 18 deletions(-) create mode 100644 eppo_client/types.py diff --git a/eppo_client/client.py b/eppo_client/client.py index cb864e1..848470d 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -1,7 +1,7 @@ import datetime import logging import json -from typing import Any, Dict, Optional, Union +from typing import Any, Dict, Optional from typing_extensions import deprecated from eppo_client.assignment_logger import AssignmentLogger from eppo_client.configuration_requestor import ( @@ -11,15 +11,13 @@ from eppo_client.models import VariationType from eppo_client.poller import Poller from eppo_client.sharding import MD5Sharder +from eppo_client.types import SubjectAttributes from eppo_client.validation import validate_not_blank from eppo_client.eval import FlagEvaluation, Evaluator logger = logging.getLogger(__name__) -AttributeValue = Union[str, float, int, bool] -SubjectAttributes = Dict[str, AttributeValue] - class EppoClient: def __init__( diff --git a/eppo_client/eval.py b/eppo_client/eval.py index d860713..9c9c35b 100644 --- a/eppo_client/eval.py +++ b/eppo_client/eval.py @@ -1,16 +1,18 @@ -from typing import Dict, Optional, Union +from typing import Dict, Optional from eppo_client.sharding import Sharder from eppo_client.models import Flag, Range, Shard, Variation from eppo_client.rules import matches_rule from dataclasses import dataclass import datetime +from eppo_client.types import SubjectAttributes + @dataclass class FlagEvaluation: flag_key: str subject_key: str - subject_attributes: Dict[str, Union[str, float, int, bool]] + subject_attributes: SubjectAttributes allocation_key: Optional[str] variation: Optional[Variation] extra_logging: Dict[str, str] @@ -25,7 +27,7 @@ def evaluate_flag( self, flag: Flag, subject_key: str, - subject_attributes: Dict[str, Union[str, float, int, bool]], + subject_attributes: SubjectAttributes, ) -> FlagEvaluation: if not flag.enabled: return none_result(flag.key, subject_key, subject_attributes) @@ -77,7 +79,9 @@ def hash_key(salt: str, subject_key: str) -> str: return f"{salt}-{subject_key}" -def none_result(flag_key, subject_key, subject_attributes) -> FlagEvaluation: +def none_result( + flag_key: str, subject_key: str, subject_attributes: SubjectAttributes +) -> FlagEvaluation: return FlagEvaluation( flag_key=flag_key, subject_key=subject_key, diff --git a/eppo_client/models.py b/eppo_client/models.py index 64fd074..6b9f073 100644 --- a/eppo_client/models.py +++ b/eppo_client/models.py @@ -1,10 +1,11 @@ from datetime import datetime from enum import Enum -from typing import Dict, List, Optional, Union +from typing import Dict, List, Optional from eppo_client.base_model import SdkBaseModel from eppo_client.rules import Rule +from eppo_client.types import ValueType class VariationType(Enum): @@ -15,9 +16,6 @@ class VariationType(Enum): JSON = "json" -ValueType = Union[str, int, float, bool] - - class Variation(SdkBaseModel): key: str value: ValueType diff --git a/eppo_client/rules.py b/eppo_client/rules.py index a7e2fa6..270e982 100644 --- a/eppo_client/rules.py +++ b/eppo_client/rules.py @@ -2,9 +2,10 @@ import re import semver from enum import Enum -from typing import Any, List, Dict, Union +from typing import Any, List from eppo_client.models import SdkBaseModel +from eppo_client.types import AttributeType, ConditionValueType, SubjectAttributes class OperatorType(Enum): @@ -20,16 +21,14 @@ class OperatorType(Enum): class Condition(SdkBaseModel): operator: OperatorType attribute: Any - value: Any = None + value: ConditionValueType class Rule(SdkBaseModel): conditions: List[Condition] -def matches_rule( - rule: Rule, subject_attributes: Dict[str, Union[str, float, int, bool]] -) -> bool: +def matches_rule(rule: Rule, subject_attributes: SubjectAttributes) -> bool: return all( evaluate_condition(condition, subject_attributes) for condition in rule.conditions @@ -37,7 +36,7 @@ def matches_rule( def evaluate_condition( - condition: Condition, subject_attributes: Dict[str, Union[str, float, int, bool]] + condition: Condition, subject_attributes: SubjectAttributes ) -> bool: subject_value = subject_attributes.get(condition.attribute, None) if subject_value is not None: diff --git a/eppo_client/types.py b/eppo_client/types.py new file mode 100644 index 0000000..26676b9 --- /dev/null +++ b/eppo_client/types.py @@ -0,0 +1,6 @@ +from typing import List, Union, Dict + +ValueType = Union[str, int, float, bool] +AttributeType = Union[str, int, float, bool] +ConditionValueType = Union[AttributeType, List[AttributeType]] +SubjectAttributes = Dict[str, AttributeType] From 6a0f3d3d3c184c15fc45a087622c8b7f32e49f10 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Thu, 21 Mar 2024 15:52:47 -0700 Subject: [PATCH 23/40] use numeric instead of float --- eppo_client/client.py | 33 ++------------------------------- eppo_client/models.py | 2 +- test/client_test.py | 15 +++++++++------ 3 files changed, 12 insertions(+), 38 deletions(-) diff --git a/eppo_client/client.py b/eppo_client/client.py index 848470d..ec08a8f 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -67,22 +67,6 @@ def get_integer_assignment( default=default, ) - def get_float_assignment( - self, - subject_key: str, - flag_key: str, - subject_attributes: Optional[SubjectAttributes] = None, - default=None, - ) -> Optional[float]: - return self.get_assignment_variation( - subject_key, - flag_key, - subject_attributes, - VariationType.FLOAT, - default=default, - ) - - @deprecated("get_numeric_assignment is deprecated in favor of get_float_assignment") def get_numeric_assignment( self, subject_key: str, @@ -90,10 +74,11 @@ def get_numeric_assignment( subject_attributes: Optional[SubjectAttributes] = None, default=None, ) -> Optional[float]: - return self.get_float_assignment( + return self.get_assignment_variation( subject_key, flag_key, subject_attributes, + VariationType.NUMERIC, default=default, ) @@ -130,20 +115,6 @@ def get_parsed_json_assignment( return None return json.loads(variation_jsons) - @deprecated( - "get_assignment is deprecated in favor of the typed get__assignment methods" - ) - def get_assignment( - self, - subject_key: str, - flag_key: str, - subject_attributes: Optional[SubjectAttributes] = None, - default=None, - ) -> Optional[str]: - return self.get_assignment_variation( - subject_key, flag_key, subject_attributes, default=default - ) - def get_assignment_variation( self, subject_key: str, diff --git a/eppo_client/models.py b/eppo_client/models.py index 6b9f073..80db7f0 100644 --- a/eppo_client/models.py +++ b/eppo_client/models.py @@ -11,7 +11,7 @@ class VariationType(Enum): STRING = "string" INTEGER = "integer" - FLOAT = "float" + NUMERIC = "numeric" BOOLEAN = "boolean" JSON = "json" diff --git a/test/client_test.py b/test/client_test.py index 2eadf03..130caf0 100644 --- a/test/client_test.py +++ b/test/client_test.py @@ -144,7 +144,11 @@ def test_with_null_experiment_config(mock_config_requestor): client = EppoClient( config_requestor=mock_config_requestor, assignment_logger=AssignmentLogger() ) - assert client.get_assignment("user-1", "flag-key-1") is None + assert client.get_string_assignment("user-1", "flag-key-1") is None + assert ( + client.get_string_assignment("user-1", "flag-key-1", default="hello world") + == "hello world" + ) @patch("eppo_client.configuration_requestor.ExperimentConfigurationRequestor") @@ -158,9 +162,9 @@ def test_graceful_mode_on(get_assignment_detail, mock_config_requestor): is_graceful_mode=True, ) - assert client.get_assignment("user-1", "experiment-key-1") is None + assert client.get_assignment_variation("user-1", "experiment-key-1") is None assert client.get_boolean_assignment("user-1", "experiment-key-1", default=True) - assert client.get_float_assignment("user-1", "experiment-key-1") is None + assert client.get_numeric_assignment("user-1", "experiment-key-1") is None assert ( client.get_string_assignment("user-1", "experiment-key-1", default="control") == "control" @@ -180,7 +184,6 @@ def test_graceful_mode_off(mock_get_assignment_detail, mock_config_requestor): ) with pytest.raises(Exception): - client.get_assignment("user-1", "experiment-key-1") client.get_boolean_assignment("user-1", "experiment-key-1") client.get_numeric_assignment("user-1", "experiment-key-1") client.get_string_assignment("user-1", "experiment-key-1") @@ -200,7 +203,7 @@ def test_assign_subject_in_sample(test_case): get_typed_assignment = { "string": client.get_string_assignment, "integer": client.get_integer_assignment, - "float": client.get_float_assignment, + "float": client.get_numeric_assignment, "boolean": client.get_boolean_assignment, "json": client.get_parsed_json_assignment, }[test_case["variationType"]] @@ -231,7 +234,7 @@ def test_get_numeric_assignment_on_bool_feature_flag_should_return_none(test_cas client = get_instance() if test_case["variationType"] == "boolean": assignments = get_assignments( - test_case=test_case, get_assignment_fn=client.get_float_assignment + test_case=test_case, get_assignment_fn=client.get_numeric_assignment ) for _, assigned_variation in assignments: assert assigned_variation is None From 6ec21dd864e9b8851bef4e5478fce21d2b727d3e Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Thu, 21 Mar 2024 16:11:14 -0700 Subject: [PATCH 24/40] more typing --- eppo_client/client.py | 1 - eppo_client/rules.py | 38 ++++++++++++++++++++++++-------------- eppo_client/types.py | 2 +- 3 files changed, 25 insertions(+), 16 deletions(-) diff --git a/eppo_client/client.py b/eppo_client/client.py index ec08a8f..20d3855 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -2,7 +2,6 @@ import logging import json from typing import Any, Dict, Optional -from typing_extensions import deprecated from eppo_client.assignment_logger import AssignmentLogger from eppo_client.configuration_requestor import ( ExperimentConfigurationRequestor, diff --git a/eppo_client/rules.py b/eppo_client/rules.py index 270e982..b9a8bc8 100644 --- a/eppo_client/rules.py +++ b/eppo_client/rules.py @@ -5,7 +5,7 @@ from typing import Any, List from eppo_client.models import SdkBaseModel -from eppo_client.types import AttributeType, ConditionValueType, SubjectAttributes +from eppo_client.types import ConditionValueType, SubjectAttributes class OperatorType(Enum): @@ -41,15 +41,17 @@ def evaluate_condition( subject_value = subject_attributes.get(condition.attribute, None) if subject_value is not None: if condition.operator == OperatorType.MATCHES: - return bool(re.match(condition.value, str(subject_value))) + return isinstance(condition.value, str) and bool( + re.match(condition.value, str(subject_value)) + ) elif condition.operator == OperatorType.ONE_OF: - return str(subject_value).lower() in [ + return isinstance(condition.value, list) and str(subject_value).lower() in [ value.lower() for value in condition.value ] elif condition.operator == OperatorType.NOT_ONE_OF: - return str(subject_value).lower() not in [ - value.lower() for value in condition.value - ] + return isinstance(condition.value, list) and str( + subject_value + ).lower() not in [value.lower() for value in condition.value] else: # Numeric operator: value could be numeric or semver. if isinstance(subject_value, numbers.Number): @@ -61,20 +63,26 @@ def evaluate_condition( return False -def evaluate_numeric_condition(subject_value: numbers.Number, condition: Condition): - if condition.operator == OperatorType.GT: - return subject_value > condition.value +def evaluate_numeric_condition( + subject_value: numbers.Number, condition: Condition +) -> bool: + if not isinstance(condition.value, numbers.Number): + # this ensures we are comparing numbers to numbers below + # but mypy is not smart enough to tell, so we ignore types below + return False + elif condition.operator == OperatorType.GT: + return subject_value > condition.value # type: ignore elif condition.operator == OperatorType.GTE: - return subject_value >= condition.value + return subject_value >= condition.value # type: ignore elif condition.operator == OperatorType.LT: - return subject_value < condition.value + return subject_value < condition.value # type: ignore elif condition.operator == OperatorType.LTE: - return subject_value <= condition.value + return subject_value <= condition.value # type: ignore return False -def is_valid_semver(value: str): +def is_valid_semver(value: str) -> bool: try: # Parse the string. If it's a valid semver, it will return without errors. semver.VersionInfo.parse(value) @@ -84,7 +92,9 @@ def is_valid_semver(value: str): return False -def compare_semver(attribute_value: Any, condition_value: Any, operator: OperatorType): +def compare_semver( + attribute_value: Any, condition_value: Any, operator: OperatorType +) -> bool: if not is_valid_semver(attribute_value) or not is_valid_semver(condition_value): return False diff --git a/eppo_client/types.py b/eppo_client/types.py index 26676b9..aba2990 100644 --- a/eppo_client/types.py +++ b/eppo_client/types.py @@ -2,5 +2,5 @@ ValueType = Union[str, int, float, bool] AttributeType = Union[str, int, float, bool] -ConditionValueType = Union[AttributeType, List[AttributeType]] +ConditionValueType = Union[AttributeType, List[str]] SubjectAttributes = Dict[str, AttributeType] From e55e88fb0733008820b190f994cd4f3e6b232bcd Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Thu, 21 Mar 2024 16:20:55 -0700 Subject: [PATCH 25/40] more careful about rules and tests --- eppo_client/rules.py | 4 ++-- eppo_client/types.py | 2 +- test/rules_test.py | 21 +++++++++++++++++++++ 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/eppo_client/rules.py b/eppo_client/rules.py index b9a8bc8..c65e4ec 100644 --- a/eppo_client/rules.py +++ b/eppo_client/rules.py @@ -46,12 +46,12 @@ def evaluate_condition( ) elif condition.operator == OperatorType.ONE_OF: return isinstance(condition.value, list) and str(subject_value).lower() in [ - value.lower() for value in condition.value + str(value).lower() for value in condition.value ] elif condition.operator == OperatorType.NOT_ONE_OF: return isinstance(condition.value, list) and str( subject_value - ).lower() not in [value.lower() for value in condition.value] + ).lower() not in [str(value).lower() for value in condition.value] else: # Numeric operator: value could be numeric or semver. if isinstance(subject_value, numbers.Number): diff --git a/eppo_client/types.py b/eppo_client/types.py index aba2990..26676b9 100644 --- a/eppo_client/types.py +++ b/eppo_client/types.py @@ -2,5 +2,5 @@ ValueType = Union[str, int, float, bool] AttributeType = Union[str, int, float, bool] -ConditionValueType = Union[AttributeType, List[str]] +ConditionValueType = Union[AttributeType, List[AttributeType]] SubjectAttributes = Dict[str, AttributeType] diff --git a/test/rules_test.py b/test/rules_test.py index 1d82e66..5fb3b9d 100644 --- a/test/rules_test.py +++ b/test/rules_test.py @@ -198,6 +198,27 @@ def test_evaluate_condition_semver(): ) +def test_evaluate_condition_one_of_int(): + one_of_condition_int = Condition( + operator=OperatorType.ONE_OF, value=[10, 20, 30], attribute="number" + ) + assert evaluate_condition(one_of_condition_int, {"number": 20}) + assert not evaluate_condition(one_of_condition_int, {"number": 40}) + assert not evaluate_condition(one_of_condition_int, {}) + + +def test_evaluate_condition_one_of_boolean(): + one_of_condition_boolean = Condition( + operator=OperatorType.ONE_OF, value=[True, False], attribute="status" + ) + assert evaluate_condition(one_of_condition_boolean, {"status": False}) + assert evaluate_condition(one_of_condition_boolean, {"status": "False"}) + assert not evaluate_condition(one_of_condition_boolean, {"status": "Maybe"}) + assert not evaluate_condition(one_of_condition_boolean, {"status": 0}) + assert not evaluate_condition(one_of_condition_boolean, {"status": 1}) + assert not evaluate_condition(one_of_condition_boolean, {}) + + def test_one_of_operator_with_number(): one_of_condition = Condition( operator=OperatorType.ONE_OF, value=["14", "15.11"], attribute="number" From 5cb9830d49276dbbdb4934a31b2c383b249453b5 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Thu, 21 Mar 2024 16:45:37 -0700 Subject: [PATCH 26/40] Address Giorgio's comments --- eppo_client/client.py | 23 +++++++++++++---------- eppo_client/configuration_requestor.py | 9 +++++---- eppo_client/eval.py | 23 +++++++++++++++++------ eppo_client/{sharding.py => sharders.py} | 0 test/eval_test.py | 6 +++++- 5 files changed, 40 insertions(+), 21 deletions(-) rename eppo_client/{sharding.py => sharders.py} (100%) diff --git a/eppo_client/client.py b/eppo_client/client.py index 20d3855..80df16d 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -9,7 +9,7 @@ from eppo_client.constants import POLL_INTERVAL_MILLIS, POLL_JITTER_MILLIS from eppo_client.models import VariationType from eppo_client.poller import Poller -from eppo_client.sharding import MD5Sharder +from eppo_client.sharders import MD5Sharder from eppo_client.types import SubjectAttributes from eppo_client.validation import validate_not_blank from eppo_client.eval import FlagEvaluation, Evaluator @@ -103,16 +103,16 @@ def get_parsed_json_assignment( subject_attributes: Optional[SubjectAttributes] = None, default=None, ) -> Optional[Dict[Any, Any]]: - variation_jsons = self.get_assignment_variation( + variation_json_string = self.get_assignment_variation( subject_key, flag_key, subject_attributes, VariationType.JSON, default=default, ) - if variation_jsons is None: + if variation_json_string is None: return None - return json.loads(variation_jsons) + return json.loads(variation_json_string) def get_assignment_variation( self, @@ -145,13 +145,14 @@ def get_assignment_detail( subject_attributes: Optional[SubjectAttributes] = None, expected_variation_type: Optional[VariationType] = None, ) -> Optional[FlagEvaluation]: - """Maps a subject to a variation for a given experiment - Returns None if the subject is not part of the experiment sample. + """Maps a subject to a variation for a given flag + Returns None if the subject is not allocated in the flag - :param subject_key: an identifier of the experiment subject, for example a user ID. - :param flag_key: an experiment or feature flag identifier + :param subject_key: an identifier of the subject, for example a user ID. + :param flag_key: a feature flag identifier :param subject_attributes: optional attributes associated with the subject, for example name and email. - The subject attributes are used for evaluating any targeting rules tied to the experiment. + The subject attributes are used for evaluating any targeting rules tied + to the flag and logged in the logging callback. """ validate_not_blank("subject_key", subject_key) validate_not_blank("flag_key", flag_key) @@ -161,7 +162,9 @@ def get_assignment_detail( flag = self.__config_requestor.get_configuration(flag_key) if flag is None: - logger.info("[Eppo SDK] No assigned variation. Flag not found: " + flag_key) + logger.warning( + "[Eppo SDK] No assigned variation. Flag not found: " + flag_key + ) return None if not check_type_match(expected_variation_type, flag.variation_type): diff --git a/eppo_client/configuration_requestor.py b/eppo_client/configuration_requestor.py index 9f468f6..4960870 100644 --- a/eppo_client/configuration_requestor.py +++ b/eppo_client/configuration_requestor.py @@ -29,11 +29,12 @@ def get_flag_keys(self): def fetch_and_store_configurations(self) -> Dict[str, Flag]: try: - configs = cast(dict, self.__http_client.get(UFC_ENDPOINT).get("flags", {})) - for flag_key, flag_config in configs.items(): - configs[flag_key] = Flag(**flag_config) + configs_dict = cast( + dict, self.__http_client.get(UFC_ENDPOINT).get("flags", {}) + ) + configs = {key: Flag(**config) for key, config in configs_dict.items()} self.__config_store.set_configurations(configs) return configs except Exception as e: - logger.error("Error retrieving assignment configurations: " + str(e)) + logger.error("Error retrieving flag configurations: " + str(e)) return {} diff --git a/eppo_client/eval.py b/eppo_client/eval.py index 9c9c35b..f1a2c1b 100644 --- a/eppo_client/eval.py +++ b/eppo_client/eval.py @@ -1,16 +1,17 @@ from typing import Dict, Optional -from eppo_client.sharding import Sharder -from eppo_client.models import Flag, Range, Shard, Variation +from eppo_client.sharders import Sharder +from eppo_client.models import Flag, Range, Shard, Variation, VariationType from eppo_client.rules import matches_rule from dataclasses import dataclass import datetime -from eppo_client.types import SubjectAttributes +from eppo_client.types import SubjectAttributes, ValueType @dataclass class FlagEvaluation: flag_key: str + variation_type: VariationType subject_key: str subject_attributes: SubjectAttributes allocation_key: Optional[str] @@ -30,7 +31,9 @@ def evaluate_flag( subject_attributes: SubjectAttributes, ) -> FlagEvaluation: if not flag.enabled: - return none_result(flag.key, subject_key, subject_attributes) + return none_result( + flag.key, flag.variation_type, subject_key, subject_attributes + ) now = utcnow() for allocation in flag.allocations: @@ -55,6 +58,7 @@ def evaluate_flag( ): return FlagEvaluation( flag_key=flag.key, + variation_type=flag.variation_type, subject_key=subject_key, subject_attributes=subject_attributes, allocation_key=allocation.key, @@ -64,9 +68,12 @@ def evaluate_flag( ) # No allocations matched, return the None result - return none_result(flag.key, subject_key, subject_attributes) + return none_result( + flag.key, flag.variation_type, subject_key, subject_attributes + ) def matches_shard(self, shard: Shard, subject_key: str, total_shards: int) -> bool: + assert total_shards > 0, "Expect total_shards to be strictly positive" h = self.sharder.get_shard(hash_key(shard.salt, subject_key), total_shards) return any(is_in_shard_range(h, r) for r in shard.ranges) @@ -80,10 +87,14 @@ def hash_key(salt: str, subject_key: str) -> str: def none_result( - flag_key: str, subject_key: str, subject_attributes: SubjectAttributes + flag_key: str, + variation_type: ValueType, + subject_key: str, + subject_attributes: SubjectAttributes, ) -> FlagEvaluation: return FlagEvaluation( flag_key=flag_key, + variation_type=variation_type, subject_key=subject_key, subject_attributes=subject_attributes, allocation_key=None, diff --git a/eppo_client/sharding.py b/eppo_client/sharders.py similarity index 100% rename from eppo_client/sharding.py rename to eppo_client/sharders.py diff --git a/test/eval_test.py b/test/eval_test.py index 77cd9bb..0c0d6e0 100644 --- a/test/eval_test.py +++ b/test/eval_test.py @@ -11,7 +11,7 @@ ) from eppo_client.eval import Evaluator, FlagEvaluation, is_in_shard_range, hash_key from eppo_client.rules import Condition, OperatorType, Rule -from eppo_client.sharding import DeterministicSharder, MD5Sharder +from eppo_client.sharders import DeterministicSharder, MD5Sharder VARIATION_A = Variation(key="a", value="A") VARIATION_B = Variation(key="b", value="B") @@ -59,6 +59,9 @@ def test_matches_shard_full_range_split(): evaluator = Evaluator(sharder=MD5Sharder()) assert evaluator.matches_shard(shard, "subject_key", 100) is True + deterministic_evaluator = Evaluator(sharder=DeterministicSharder({"subject": 50})) + assert deterministic_evaluator.matches_shard(shard, "subject_key", 100) is True + def test_matches_shard_no_match(): shard = Shard( @@ -86,6 +89,7 @@ def test_eval_empty_flag(): evaluator = Evaluator(sharder=MD5Sharder()) assert evaluator.evaluate_flag(empty_flag, "subject_key", {}) == FlagEvaluation( flag_key="empty", + variation_type=VariationType.STRING, subject_key="subject_key", subject_attributes={}, allocation_key=None, From 7897a674cb0aa469da332d9ed7a0123177f27f16 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Thu, 21 Mar 2024 19:30:28 -0700 Subject: [PATCH 27/40] fix typing error in none_result --- eppo_client/eval.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eppo_client/eval.py b/eppo_client/eval.py index f1a2c1b..ec8df92 100644 --- a/eppo_client/eval.py +++ b/eppo_client/eval.py @@ -88,7 +88,7 @@ def hash_key(salt: str, subject_key: str) -> str: def none_result( flag_key: str, - variation_type: ValueType, + variation_type: VariationType, subject_key: str, subject_attributes: SubjectAttributes, ) -> FlagEvaluation: From 0506aaa8f8151a442a320cd65f45ca546fab86c7 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Fri, 22 Mar 2024 16:08:26 -0700 Subject: [PATCH 28/40] add NOT_MATCHES operator --- eppo_client/eval.py | 2 +- eppo_client/rules.py | 5 +++++ test/rules_test.py | 21 +++++++++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/eppo_client/eval.py b/eppo_client/eval.py index ec8df92..0bd9976 100644 --- a/eppo_client/eval.py +++ b/eppo_client/eval.py @@ -5,7 +5,7 @@ from dataclasses import dataclass import datetime -from eppo_client.types import SubjectAttributes, ValueType +from eppo_client.types import SubjectAttributes @dataclass diff --git a/eppo_client/rules.py b/eppo_client/rules.py index c65e4ec..fb2f9f0 100644 --- a/eppo_client/rules.py +++ b/eppo_client/rules.py @@ -10,6 +10,7 @@ class OperatorType(Enum): MATCHES = "MATCHES" + NOT_MATCHES = "NOT_MATCHES" GTE = "GTE" GT = "GT" LTE = "LTE" @@ -44,6 +45,10 @@ def evaluate_condition( return isinstance(condition.value, str) and bool( re.match(condition.value, str(subject_value)) ) + if condition.operator == OperatorType.NOT_MATCHES: + return isinstance(condition.value, str) and not bool( + re.match(condition.value, str(subject_value)) + ) elif condition.operator == OperatorType.ONE_OF: return isinstance(condition.value, list) and str(subject_value).lower() in [ str(value).lower() for value in condition.value diff --git a/test/rules_test.py b/test/rules_test.py index 5fb3b9d..23d90c8 100644 --- a/test/rules_test.py +++ b/test/rules_test.py @@ -143,6 +143,27 @@ def test_evaluate_condition_matches(): ) +def test_evaluate_condition_not_matches(): + assert not evaluate_condition( + Condition( + operator=OperatorType.NOT_MATCHES, value="^test.*", attribute="email" + ), + {"email": "test@example.com"}, + ) + assert not evaluate_condition( + Condition( + operator=OperatorType.NOT_MATCHES, value="^test.*", attribute="email" + ), + {}, + ) + assert evaluate_condition( + Condition( + operator=OperatorType.NOT_MATCHES, value="^test.*", attribute="email" + ), + {"email": "example@test.com"}, + ) + + def test_evaluate_condition_gte(): assert evaluate_condition( Condition(operator=OperatorType.GTE, value=18, attribute="age"), From de969b028b60feb296d394c7a570024d8b1a8264 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Sat, 23 Mar 2024 13:55:48 -0700 Subject: [PATCH 29/40] get_integer_assignment should return an integer --- eppo_client/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eppo_client/client.py b/eppo_client/client.py index 80df16d..6671923 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -57,7 +57,7 @@ def get_integer_assignment( flag_key: str, subject_attributes: Optional[SubjectAttributes] = None, default=None, - ) -> Optional[float]: + ) -> Optional[int]: return self.get_assignment_variation( subject_key, flag_key, From dcbe36e29ce9e3814088a1e14264b3da967eb0f1 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Mon, 25 Mar 2024 14:44:41 -0700 Subject: [PATCH 30/40] update types in client.py --- eppo_client/client.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/eppo_client/client.py b/eppo_client/client.py index 6671923..0ba4908 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -10,7 +10,7 @@ from eppo_client.models import VariationType from eppo_client.poller import Poller from eppo_client.sharders import MD5Sharder -from eppo_client.types import SubjectAttributes +from eppo_client.types import SubjectAttributes, ValueType from eppo_client.validation import validate_not_blank from eppo_client.eval import FlagEvaluation, Evaluator @@ -41,7 +41,7 @@ def get_string_assignment( subject_key: str, flag_key: str, subject_attributes: Optional[SubjectAttributes] = None, - default=None, + default: Optional[str] = None, ) -> Optional[str]: return self.get_assignment_variation( subject_key, @@ -56,7 +56,7 @@ def get_integer_assignment( subject_key: str, flag_key: str, subject_attributes: Optional[SubjectAttributes] = None, - default=None, + default: Optional[int] = None, ) -> Optional[int]: return self.get_assignment_variation( subject_key, @@ -71,7 +71,7 @@ def get_numeric_assignment( subject_key: str, flag_key: str, subject_attributes: Optional[SubjectAttributes] = None, - default=None, + default: Optional[float] = None, ) -> Optional[float]: return self.get_assignment_variation( subject_key, @@ -86,7 +86,7 @@ def get_boolean_assignment( subject_key: str, flag_key: str, subject_attributes: Optional[SubjectAttributes] = None, - default=None, + default: Optional[bool] = None, ) -> Optional[bool]: return self.get_assignment_variation( subject_key, @@ -101,17 +101,17 @@ def get_parsed_json_assignment( subject_key: str, flag_key: str, subject_attributes: Optional[SubjectAttributes] = None, - default=None, + default: Optional[Dict[Any, Any]] = None, ) -> Optional[Dict[Any, Any]]: variation_json_string = self.get_assignment_variation( subject_key, flag_key, subject_attributes, VariationType.JSON, - default=default, + default=None, ) if variation_json_string is None: - return None + return default return json.loads(variation_json_string) def get_assignment_variation( @@ -120,7 +120,7 @@ def get_assignment_variation( flag_key: str, subject_attributes: Optional[SubjectAttributes] = None, expected_variation_type: Optional[VariationType] = None, - default=None, + default: Optional[ValueType] = None, ): try: result = self.get_assignment_detail( From b0ad9c1be3f5d150f7314af1d835933de8e0452f Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Wed, 27 Mar 2024 18:01:33 -0700 Subject: [PATCH 31/40] use numeric instead of float --- test/client_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/client_test.py b/test/client_test.py index 130caf0..56624ea 100644 --- a/test/client_test.py +++ b/test/client_test.py @@ -203,7 +203,7 @@ def test_assign_subject_in_sample(test_case): get_typed_assignment = { "string": client.get_string_assignment, "integer": client.get_integer_assignment, - "float": client.get_numeric_assignment, + "numeric": client.get_numeric_assignment, "boolean": client.get_boolean_assignment, "json": client.get_parsed_json_assignment, }[test_case["variationType"]] From 82483531958ac2640a783b27b573f57c23e29f92 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Wed, 3 Apr 2024 15:48:00 -0700 Subject: [PATCH 32/40] Fix UFC tests, force default value --- eppo_client/client.py | 34 ++++++++++++++-------------- eppo_client/models.py | 10 ++++----- test/client_test.py | 52 ++++++++++++++++++++++++++++--------------- 3 files changed, 56 insertions(+), 40 deletions(-) diff --git a/eppo_client/client.py b/eppo_client/client.py index 0ba4908..70100f1 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -40,75 +40,75 @@ def get_string_assignment( self, subject_key: str, flag_key: str, + default: str, subject_attributes: Optional[SubjectAttributes] = None, - default: Optional[str] = None, - ) -> Optional[str]: + ) -> str: return self.get_assignment_variation( subject_key, flag_key, + default, subject_attributes, VariationType.STRING, - default=default, ) def get_integer_assignment( self, subject_key: str, flag_key: str, + default: int, subject_attributes: Optional[SubjectAttributes] = None, - default: Optional[int] = None, - ) -> Optional[int]: + ) -> int: return self.get_assignment_variation( subject_key, flag_key, + default, subject_attributes, VariationType.INTEGER, - default=default, ) def get_numeric_assignment( self, subject_key: str, flag_key: str, + default: float, subject_attributes: Optional[SubjectAttributes] = None, - default: Optional[float] = None, - ) -> Optional[float]: + ) -> float: return self.get_assignment_variation( subject_key, flag_key, + default, subject_attributes, VariationType.NUMERIC, - default=default, ) def get_boolean_assignment( self, subject_key: str, flag_key: str, + default: bool, subject_attributes: Optional[SubjectAttributes] = None, - default: Optional[bool] = None, - ) -> Optional[bool]: + ) -> bool: return self.get_assignment_variation( subject_key, flag_key, + default, subject_attributes, VariationType.BOOLEAN, - default=default, ) - def get_parsed_json_assignment( + def get_json_assignment( self, subject_key: str, flag_key: str, + default: Dict[Any, Any], subject_attributes: Optional[SubjectAttributes] = None, - default: Optional[Dict[Any, Any]] = None, - ) -> Optional[Dict[Any, Any]]: + ) -> Dict[Any, Any]: variation_json_string = self.get_assignment_variation( subject_key, flag_key, + None, subject_attributes, VariationType.JSON, - default=None, ) if variation_json_string is None: return default @@ -118,9 +118,9 @@ def get_assignment_variation( self, subject_key: str, flag_key: str, + default: Optional[ValueType] = None, subject_attributes: Optional[SubjectAttributes] = None, expected_variation_type: Optional[VariationType] = None, - default: Optional[ValueType] = None, ): try: result = self.get_assignment_detail( diff --git a/eppo_client/models.py b/eppo_client/models.py index 80db7f0..a88a122 100644 --- a/eppo_client/models.py +++ b/eppo_client/models.py @@ -9,11 +9,11 @@ class VariationType(Enum): - STRING = "string" - INTEGER = "integer" - NUMERIC = "numeric" - BOOLEAN = "boolean" - JSON = "json" + STRING = "STRING" + INTEGER = "INTEGER" + NUMERIC = "NUMERIC" + BOOLEAN = "BOOLEAN" + JSON = "JSON" class Variation(SdkBaseModel): diff --git a/test/client_test.py b/test/client_test.py index 56624ea..35a8634 100644 --- a/test/client_test.py +++ b/test/client_test.py @@ -61,7 +61,7 @@ def test_assign_blank_flag_key(mock_config_requestor): config_requestor=mock_config_requestor, assignment_logger=AssignmentLogger() ) with pytest.raises(Exception) as exc_info: - client.get_string_assignment("subject-1", "") + client.get_string_assignment("subject-1", "", "default value") assert exc_info.value.args[0] == "Invalid value for flag_key: cannot be blank" @@ -71,7 +71,7 @@ def test_assign_blank_subject(mock_config_requestor): config_requestor=mock_config_requestor, assignment_logger=AssignmentLogger() ) with pytest.raises(Exception) as exc_info: - client.get_string_assignment("", "experiment-1") + client.get_string_assignment("", "experiment-1", "default value") assert exc_info.value.args[0] == "Invalid value for subject_key: cannot be blank" @@ -102,7 +102,9 @@ def test_log_assignment(mock_config_requestor, mock_logger): client = EppoClient( config_requestor=mock_config_requestor, assignment_logger=mock_logger ) - assert client.get_string_assignment("user-1", "flag-key") == "control" + assert ( + client.get_string_assignment("user-1", "flag-key", "default value") == "control" + ) assert mock_logger.log_assignment.call_count == 1 @@ -135,7 +137,9 @@ def test_get_assignment_handles_logging_exception(mock_config_requestor, mock_lo client = EppoClient( config_requestor=mock_config_requestor, assignment_logger=mock_logger ) - assert client.get_string_assignment("user-1", "flag-key") == "control" + assert ( + client.get_string_assignment("user-1", "flag-key", "default value") == "control" + ) @patch("eppo_client.configuration_requestor.ExperimentConfigurationRequestor") @@ -144,7 +148,10 @@ def test_with_null_experiment_config(mock_config_requestor): client = EppoClient( config_requestor=mock_config_requestor, assignment_logger=AssignmentLogger() ) - assert client.get_string_assignment("user-1", "flag-key-1") is None + assert ( + client.get_string_assignment("user-1", "flag-key-1", "default value") + == "default value" + ) assert ( client.get_string_assignment("user-1", "flag-key-1", default="hello world") == "hello world" @@ -162,14 +169,19 @@ def test_graceful_mode_on(get_assignment_detail, mock_config_requestor): is_graceful_mode=True, ) - assert client.get_assignment_variation("user-1", "experiment-key-1") is None + assert ( + client.get_assignment_variation("user-1", "experiment-key-1", "default") + == "default" + ) assert client.get_boolean_assignment("user-1", "experiment-key-1", default=True) - assert client.get_numeric_assignment("user-1", "experiment-key-1") is None + assert client.get_numeric_assignment("user-1", "experiment-key-1", 1.0) == 1.0 assert ( client.get_string_assignment("user-1", "experiment-key-1", default="control") == "control" ) - assert client.get_parsed_json_assignment("user-1", "experiment-key-1") is None + assert client.get_json_assignment( + "user-1", "experiment-key-1", {"hello": "world"} + ) == {"hello": "world"} @patch("eppo_client.configuration_requestor.ExperimentConfigurationRequestor") @@ -184,10 +196,11 @@ def test_graceful_mode_off(mock_get_assignment_detail, mock_config_requestor): ) with pytest.raises(Exception): - client.get_boolean_assignment("user-1", "experiment-key-1") - client.get_numeric_assignment("user-1", "experiment-key-1") - client.get_string_assignment("user-1", "experiment-key-1") - client.get_parsed_json_assignment("user-1", "experiment-key-1") + client.get_boolean_assignment("user-1", "experiment-key-1", True) + client.get_numeric_assignment("user-1", "experiment-key-1", 0.0) + client.get_integer_assignment("user-1", "experiment-key-1", 1) + client.get_string_assignment("user-1", "experiment-key-1", "default value") + client.get_json_assignment("user-1", "experiment-key-1", {"hello": "world"}) def test_client_has_flags(): @@ -201,11 +214,11 @@ def test_assign_subject_in_sample(test_case): print("---- Test case for {} Experiment".format(test_case["flag"])) get_typed_assignment = { - "string": client.get_string_assignment, - "integer": client.get_integer_assignment, - "numeric": client.get_numeric_assignment, - "boolean": client.get_boolean_assignment, - "json": client.get_parsed_json_assignment, + "STRING": client.get_string_assignment, + "INTEGER": client.get_integer_assignment, + "NUMERIC": client.get_numeric_assignment, + "BOOLEAN": client.get_boolean_assignment, + "JSON": client.get_json_assignment, }[test_case["variationType"]] assignments = get_assignments(test_case, get_typed_assignment) @@ -223,7 +236,10 @@ def get_assignments(test_case, get_assignment_fn): assignments = [] for subject in test_case.get("subjects", []): variation = get_assignment_fn( - subject["subjectKey"], test_case["flag"], subject["subjectAttributes"] + subject["subjectKey"], + test_case["flag"], + test_case["defaultValue"], + subject["subjectAttributes"], ) assignments.append((subject, variation)) return assignments From 9fda450a614fbad61b1cacd64a996dbb00e5240e Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Thu, 4 Apr 2024 15:22:05 -0700 Subject: [PATCH 33/40] add metadata logging and is_initialized --- eppo_client/__init__.py | 2 +- eppo_client/client.py | 9 +++++++++ eppo_client/configuration_requestor.py | 5 +++++ eppo_client/version.py | 1 + setup.cfg | 2 +- test/client_test.py | 5 +++++ 6 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 eppo_client/version.py diff --git a/eppo_client/__init__.py b/eppo_client/__init__.py index 4231fff..d0185aa 100644 --- a/eppo_client/__init__.py +++ b/eppo_client/__init__.py @@ -9,8 +9,8 @@ from eppo_client.http_client import HttpClient, SdkParams from eppo_client.models import Flag from eppo_client.read_write_lock import ReadWriteLock +from eppo_client.version import __version__ -__version__ = "3.0.0" __client: Optional[EppoClient] = None __lock = ReadWriteLock() diff --git a/eppo_client/client.py b/eppo_client/client.py index 70100f1..94dda31 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -13,6 +13,7 @@ from eppo_client.types import SubjectAttributes, ValueType from eppo_client.validation import validate_not_blank from eppo_client.eval import FlagEvaluation, Evaluator +from eppo_client.version import __version__ logger = logging.getLogger(__name__) @@ -190,6 +191,7 @@ def get_assignment_detail( "subject": subject_key, "timestamp": datetime.datetime.utcnow().isoformat(), "subjectAttributes": subject_attributes, + "metaData": {"sdkLanguage": "python", "sdkVersion": __version__}, } try: if result and result.do_log: @@ -207,6 +209,13 @@ def get_flag_keys(self): """ return self.__config_requestor.get_flag_keys() + def is_initialized(self): + """ + Returns True if the client has successfully initialized + the flag configuration and is ready to serve requests. + """ + return self.__config_requestor.is_initialized() + def _shutdown(self): """Stops all background processes used by the client Do not use the client after calling this method. diff --git a/eppo_client/configuration_requestor.py b/eppo_client/configuration_requestor.py index 4960870..02f035d 100644 --- a/eppo_client/configuration_requestor.py +++ b/eppo_client/configuration_requestor.py @@ -18,6 +18,7 @@ def __init__( ): self.__http_client = http_client self.__config_store = config_store + self.__is_initialized = False def get_configuration(self, flag_key: str) -> Optional[Flag]: if self.__http_client.is_unauthorized(): @@ -34,7 +35,11 @@ def fetch_and_store_configurations(self) -> Dict[str, Flag]: ) configs = {key: Flag(**config) for key, config in configs_dict.items()} self.__config_store.set_configurations(configs) + self.__is_initialized = True return configs except Exception as e: logger.error("Error retrieving flag configurations: " + str(e)) return {} + + def is_initialized(self): + return self.__is_initialized diff --git a/eppo_client/version.py b/eppo_client/version.py new file mode 100644 index 0000000..528787c --- /dev/null +++ b/eppo_client/version.py @@ -0,0 +1 @@ +__version__ = "3.0.0" diff --git a/setup.cfg b/setup.cfg index eff59e3..cf6bf4b 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [metadata] name = eppo-server-sdk -version = attr: eppo_client.__version__ +version = attr: eppo_client.version.__version__ author = Eppo author_email = eppo-team@geteppo.com description = Eppo SDK for Python diff --git a/test/client_test.py b/test/client_test.py index 35a8634..075186d 100644 --- a/test/client_test.py +++ b/test/client_test.py @@ -55,6 +55,11 @@ def init_fixture(): httpretty.disable() +def test_is_initialized(): + client = get_instance() + assert client.is_initialized() + + @patch("eppo_client.configuration_requestor.ExperimentConfigurationRequestor") def test_assign_blank_flag_key(mock_config_requestor): client = EppoClient( From 2c0b6cf949cb31d8f4daf98607412664f0cabc60 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Thu, 11 Apr 2024 21:17:07 -0700 Subject: [PATCH 34/40] update endpoint --- eppo_client/configuration_requestor.py | 2 +- test/client_test.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/eppo_client/configuration_requestor.py b/eppo_client/configuration_requestor.py index 02f035d..9b1d5dd 100644 --- a/eppo_client/configuration_requestor.py +++ b/eppo_client/configuration_requestor.py @@ -7,7 +7,7 @@ logger = logging.getLogger(__name__) -UFC_ENDPOINT = "/flag_config/v1/config" +UFC_ENDPOINT = "/flag-config/v1/config" class ExperimentConfigurationRequestor: diff --git a/test/client_test.py b/test/client_test.py index 075186d..fa39f90 100644 --- a/test/client_test.py +++ b/test/client_test.py @@ -39,7 +39,7 @@ def init_fixture(): with open(CONFIG_FILE) as mock_ufc_response: httpretty.register_uri( httpretty.GET, - MOCK_BASE_URL + "/flag_config/v1/config", + MOCK_BASE_URL + "/flag-config/v1/config", body=json.dumps(json.load(mock_ufc_response)), ) client = init( From 1b830d896f9a858a2fb23bd5e94bd8d641e50171 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Fri, 12 Apr 2024 15:50:52 -0700 Subject: [PATCH 35/40] Python UFC SDK updates (#29) * Pass new tests * mypy + flake --- eppo_client/client.py | 41 ++++++++++++++++++++++++++++++------ eppo_client/eval.py | 15 ++++++++------ eppo_client/rules.py | 6 ++++++ test/client_test.py | 15 +++++++++++++- test/eval_test.py | 48 ++++++++++++++++++++++++++++++++++++++++++- test/rules_test.py | 18 ++++++++++++++++ 6 files changed, 129 insertions(+), 14 deletions(-) diff --git a/eppo_client/client.py b/eppo_client/client.py index 94dda31..8e0c65c 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -74,12 +74,15 @@ def get_numeric_assignment( default: float, subject_attributes: Optional[SubjectAttributes] = None, ) -> float: - return self.get_assignment_variation( - subject_key, - flag_key, - default, - subject_attributes, - VariationType.NUMERIC, + # convert to float in case we get an int + return float( + self.get_assignment_variation( + subject_key, + flag_key, + default, + subject_attributes, + VariationType.NUMERIC, + ) ) def get_boolean_assignment( @@ -182,6 +185,15 @@ def get_assignment_detail( result = self.__evaluator.evaluate_flag(flag, subject_key, subject_attributes) + if result.variation and not check_value_type_match( + expected_variation_type, result.variation.value + ): + logger.error( + "[Eppo SDK] Variation value does not have the correct type for the flag: " + f"{flag_key} and variation key {result.variation.key}" + ) + return None + assignment_event = { **(result.extra_logging if result else {}), "allocation": result.allocation_key if result else None, @@ -227,3 +239,20 @@ def check_type_match( expected_type: Optional[VariationType], actual_type: VariationType ): return expected_type is None or actual_type == expected_type + + +def check_value_type_match( + expected_type: Optional[VariationType], value: ValueType +) -> bool: + if expected_type is None: + return True + if expected_type in [VariationType.JSON, VariationType.STRING]: + return isinstance(value, str) + if expected_type == VariationType.INTEGER: + return isinstance(value, int) + if expected_type == VariationType.NUMERIC: + # we can convert int to float + return isinstance(value, float) or isinstance(value, int) + if expected_type == VariationType.BOOLEAN: + return isinstance(value, bool) + return False diff --git a/eppo_client/eval.py b/eppo_client/eval.py index 0bd9976..d8350bd 100644 --- a/eppo_client/eval.py +++ b/eppo_client/eval.py @@ -43,12 +43,8 @@ def evaluate_flag( if allocation.end_at and now > allocation.end_at: continue - # Skip allocations when none of the rules match - # So we look for (rule 1) OR (rule 2) OR (rule 3) etc. - # If there are no rules, then we always match - if not allocation.rules or any( - matches_rule(rule, {"id": subject_key, **subject_attributes}) - for rule in allocation.rules + if matches_rules( + allocation.rules, {"id": subject_key, **subject_attributes} ): for split in allocation.splits: # Split needs to match all shards @@ -86,6 +82,13 @@ def hash_key(salt: str, subject_key: str) -> str: return f"{salt}-{subject_key}" +def matches_rules(rules, subject_attributes): + # Skip allocations when none of the rules match + # So we look for (rule 1) OR (rule 2) OR (rule 3) etc. + # If there are no rules, then we always match + return not rules or any(matches_rule(rule, subject_attributes) for rule in rules) + + def none_result( flag_key: str, variation_type: VariationType, diff --git a/eppo_client/rules.py b/eppo_client/rules.py index fb2f9f0..bc7c969 100644 --- a/eppo_client/rules.py +++ b/eppo_client/rules.py @@ -17,6 +17,7 @@ class OperatorType(Enum): LT = "LT" ONE_OF = "ONE_OF" NOT_ONE_OF = "NOT_ONE_OF" + IS_NULL = "IS_NULL" class Condition(SdkBaseModel): @@ -40,6 +41,11 @@ def evaluate_condition( condition: Condition, subject_attributes: SubjectAttributes ) -> bool: subject_value = subject_attributes.get(condition.attribute, None) + if condition.operator == OperatorType.IS_NULL: + if condition.value: + return subject_value is None + return subject_value is not None + if subject_value is not None: if condition.operator == OperatorType.MATCHES: return isinstance(condition.value, str) and bool( diff --git a/test/client_test.py b/test/client_test.py index fa39f90..e9aeae0 100644 --- a/test/client_test.py +++ b/test/client_test.py @@ -5,7 +5,7 @@ import httpretty # type: ignore import pytest from eppo_client.assignment_logger import AssignmentLogger -from eppo_client.client import EppoClient, check_type_match +from eppo_client.client import EppoClient, check_type_match, check_value_type_match from eppo_client.config import Config from eppo_client.models import ( Allocation, @@ -270,3 +270,16 @@ def test_get_numeric_assignment_on_bool_feature_flag_should_return_none(test_cas def test_check_type_match(): assert check_type_match(VariationType.STRING, VariationType.STRING) assert check_type_match(None, VariationType.STRING) + + +def test_check_value_type_match(): + assert check_value_type_match(VariationType.STRING, "hello") + assert check_value_type_match(VariationType.INTEGER, 1) + assert check_value_type_match(VariationType.NUMERIC, 1.0) + assert check_value_type_match(VariationType.NUMERIC, 1) + assert check_value_type_match(VariationType.BOOLEAN, True) + assert check_value_type_match(VariationType.JSON, '{"hello": "world"}') + + assert not check_type_match(VariationType.STRING, 1) + assert not check_type_match(VariationType.INTEGER, 1.0) + assert not check_type_match(VariationType.BOOLEAN, "true") diff --git a/test/eval_test.py b/test/eval_test.py index 0c0d6e0..7eb3475 100644 --- a/test/eval_test.py +++ b/test/eval_test.py @@ -9,7 +9,13 @@ Split, Shard, ) -from eppo_client.eval import Evaluator, FlagEvaluation, is_in_shard_range, hash_key +from eppo_client.eval import ( + Evaluator, + FlagEvaluation, + is_in_shard_range, + hash_key, + matches_rules, +) from eppo_client.rules import Condition, OperatorType, Rule from eppo_client.sharders import DeterministicSharder, MD5Sharder @@ -424,6 +430,46 @@ def test_eval_after_alloc(mocker): assert result.variation is None +def test_matches_rules_empty(): + rules = [] + subject_attributes = {"size": 10} + assert matches_rules(rules, subject_attributes) + + +def test_matches_rules_with_conditions(): + rules = [ + Rule( + conditions=[ + Condition(attribute="size", operator=OperatorType.IS_NULL, value=True) + ] + ), + Rule( + conditions=[ + Condition( + attribute="country", operator=OperatorType.ONE_OF, value=["UK"] + ) + ] + ), + ] + subject_attributes_1 = {"size": None, "country": "US"} + subject_attributes_2 = {"size": 10, "country": "UK"} + subject_attributes_3 = {"size": 5, "country": "US"} + subject_attributes_4 = {"country": "US"} + + assert ( + matches_rules(rules, subject_attributes_1) is True + ), f"{subject_attributes_1} should match first rule" + assert ( + matches_rules(rules, subject_attributes_2) is True + ), f"{subject_attributes_2} should match second rule" + assert ( + matches_rules(rules, subject_attributes_3) is False + ), f"{subject_attributes_3} should not match any rule" + assert ( + matches_rules(rules, subject_attributes_4) is True + ), f"{subject_attributes_4} should match first rule" + + def test_seed(): assert hash_key("salt", "subject") == "salt-subject" diff --git a/test/rules_test.py b/test/rules_test.py index 23d90c8..4c67079 100644 --- a/test/rules_test.py +++ b/test/rules_test.py @@ -255,3 +255,21 @@ def test_one_of_operator_with_number(): assert not evaluate_condition(not_one_of_condition, {"number": 10}) assert evaluate_condition(not_one_of_condition, {"number": "11"}) assert evaluate_condition(not_one_of_condition, {"number": 11}) + + +def test_is_null_operator(): + is_null_condition = Condition( + operator=OperatorType.IS_NULL, value=True, attribute="size" + ) + assert evaluate_condition(is_null_condition, {"size": None}) + assert not evaluate_condition(is_null_condition, {"size": 10}) + assert evaluate_condition(is_null_condition, {}) + + +def test_is_not_null_operator(): + is_not_null_condition = Condition( + operator=OperatorType.IS_NULL, value=False, attribute="size" + ) + assert not evaluate_condition(is_not_null_condition, {"size": None}) + assert evaluate_condition(is_not_null_condition, {"size": 10}) + assert not evaluate_condition(is_not_null_condition, {}) From 39315507505d5c31ee70696208a3812437242c3b Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Wed, 17 Apr 2024 21:45:40 -0700 Subject: [PATCH 36/40] Update function signatures --- eppo_client/client.py | 50 +++++++++++++++++++++---------------------- eppo_client/eval.py | 2 +- test/client_test.py | 41 +++++++++++++++++++---------------- 3 files changed, 48 insertions(+), 45 deletions(-) diff --git a/eppo_client/client.py b/eppo_client/client.py index 8e0c65c..d5904e2 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -39,79 +39,79 @@ def __init__( def get_string_assignment( self, - subject_key: str, flag_key: str, + subject_key: str, + subject_attributes: SubjectAttributes, default: str, - subject_attributes: Optional[SubjectAttributes] = None, ) -> str: return self.get_assignment_variation( - subject_key, flag_key, - default, + subject_key, subject_attributes, + default, VariationType.STRING, ) def get_integer_assignment( self, - subject_key: str, flag_key: str, + subject_key: str, + subject_attributes: SubjectAttributes, default: int, - subject_attributes: Optional[SubjectAttributes] = None, ) -> int: return self.get_assignment_variation( - subject_key, flag_key, - default, + subject_key, subject_attributes, + default, VariationType.INTEGER, ) def get_numeric_assignment( self, - subject_key: str, flag_key: str, + subject_key: str, + subject_attributes: SubjectAttributes, default: float, - subject_attributes: Optional[SubjectAttributes] = None, ) -> float: # convert to float in case we get an int return float( self.get_assignment_variation( - subject_key, flag_key, - default, + subject_key, subject_attributes, + default, VariationType.NUMERIC, ) ) def get_boolean_assignment( self, - subject_key: str, flag_key: str, + subject_key: str, + subject_attributes: SubjectAttributes, default: bool, - subject_attributes: Optional[SubjectAttributes] = None, ) -> bool: return self.get_assignment_variation( - subject_key, flag_key, - default, + subject_key, subject_attributes, + default, VariationType.BOOLEAN, ) def get_json_assignment( self, - subject_key: str, flag_key: str, + subject_key: str, + subject_attributes: SubjectAttributes, default: Dict[Any, Any], - subject_attributes: Optional[SubjectAttributes] = None, ) -> Dict[Any, Any]: variation_json_string = self.get_assignment_variation( - subject_key, flag_key, - None, + subject_key, subject_attributes, + None, VariationType.JSON, ) if variation_json_string is None: @@ -120,15 +120,15 @@ def get_json_assignment( def get_assignment_variation( self, - subject_key: str, flag_key: str, + subject_key: str, + subject_attributes: SubjectAttributes, default: Optional[ValueType] = None, - subject_attributes: Optional[SubjectAttributes] = None, expected_variation_type: Optional[VariationType] = None, ): try: result = self.get_assignment_detail( - subject_key, flag_key, subject_attributes, expected_variation_type + flag_key, subject_key, subject_attributes, expected_variation_type ) if not result or not result.variation: return default @@ -144,9 +144,9 @@ def get_assignment_variation( def get_assignment_detail( self, - subject_key: str, flag_key: str, - subject_attributes: Optional[SubjectAttributes] = None, + subject_key: str, + subject_attributes: SubjectAttributes, expected_variation_type: Optional[VariationType] = None, ) -> Optional[FlagEvaluation]: """Maps a subject to a variation for a given flag diff --git a/eppo_client/eval.py b/eppo_client/eval.py index d8350bd..ceaa907 100644 --- a/eppo_client/eval.py +++ b/eppo_client/eval.py @@ -108,4 +108,4 @@ def none_result( def utcnow() -> datetime.datetime: - return datetime.datetime.utcnow() + return datetime.datetime.now(datetime.timezone.utc) diff --git a/test/client_test.py b/test/client_test.py index e9aeae0..e34ea04 100644 --- a/test/client_test.py +++ b/test/client_test.py @@ -66,7 +66,7 @@ def test_assign_blank_flag_key(mock_config_requestor): config_requestor=mock_config_requestor, assignment_logger=AssignmentLogger() ) with pytest.raises(Exception) as exc_info: - client.get_string_assignment("subject-1", "", "default value") + client.get_string_assignment("", "subject-1", {}, "default value") assert exc_info.value.args[0] == "Invalid value for flag_key: cannot be blank" @@ -76,7 +76,7 @@ def test_assign_blank_subject(mock_config_requestor): config_requestor=mock_config_requestor, assignment_logger=AssignmentLogger() ) with pytest.raises(Exception) as exc_info: - client.get_string_assignment("", "experiment-1", "default value") + client.get_string_assignment("experiment-1", "", {}, "default value") assert exc_info.value.args[0] == "Invalid value for subject_key: cannot be blank" @@ -91,7 +91,6 @@ def test_log_assignment(mock_config_requestor, mock_logger): allocations=[ Allocation( key="allocation", - rules=[], splits=[ Split( variation_key="control", @@ -108,7 +107,8 @@ def test_log_assignment(mock_config_requestor, mock_logger): config_requestor=mock_config_requestor, assignment_logger=mock_logger ) assert ( - client.get_string_assignment("user-1", "flag-key", "default value") == "control" + client.get_string_assignment("falg-key", "user-1", {}, "default value") + == "control" ) assert mock_logger.log_assignment.call_count == 1 @@ -143,7 +143,8 @@ def test_get_assignment_handles_logging_exception(mock_config_requestor, mock_lo config_requestor=mock_config_requestor, assignment_logger=mock_logger ) assert ( - client.get_string_assignment("user-1", "flag-key", "default value") == "control" + client.get_string_assignment("flag-key", "user-1", {}, "default value") + == "control" ) @@ -154,11 +155,11 @@ def test_with_null_experiment_config(mock_config_requestor): config_requestor=mock_config_requestor, assignment_logger=AssignmentLogger() ) assert ( - client.get_string_assignment("user-1", "flag-key-1", "default value") + client.get_string_assignment("flag-key-1", "user-1", {}, "default value") == "default value" ) assert ( - client.get_string_assignment("user-1", "flag-key-1", default="hello world") + client.get_string_assignment("flag-key-1", "user-1", {}, "hello world") == "hello world" ) @@ -175,17 +176,19 @@ def test_graceful_mode_on(get_assignment_detail, mock_config_requestor): ) assert ( - client.get_assignment_variation("user-1", "experiment-key-1", "default") + client.get_assignment_variation("experiment-key-1", "user-1", {}, "default") == "default" ) - assert client.get_boolean_assignment("user-1", "experiment-key-1", default=True) - assert client.get_numeric_assignment("user-1", "experiment-key-1", 1.0) == 1.0 + assert client.get_boolean_assignment("experiment-key-1", "user-1", {}, default=True) + assert client.get_numeric_assignment("experiment-key-1", "user-1", {}, 1.0) == 1.0 assert ( - client.get_string_assignment("user-1", "experiment-key-1", default="control") + client.get_string_assignment( + "experiment-key-1", "user-1", {}, default="control" + ) == "control" ) assert client.get_json_assignment( - "user-1", "experiment-key-1", {"hello": "world"} + "experiment-key-1", "user-1", {}, {"hello": "world"} ) == {"hello": "world"} @@ -201,11 +204,11 @@ def test_graceful_mode_off(mock_get_assignment_detail, mock_config_requestor): ) with pytest.raises(Exception): - client.get_boolean_assignment("user-1", "experiment-key-1", True) - client.get_numeric_assignment("user-1", "experiment-key-1", 0.0) - client.get_integer_assignment("user-1", "experiment-key-1", 1) - client.get_string_assignment("user-1", "experiment-key-1", "default value") - client.get_json_assignment("user-1", "experiment-key-1", {"hello": "world"}) + client.get_boolean_assignment("experiment-key-1", "user-1", {}, True) + client.get_numeric_assignment("experiment-key-1", "user-1", {}, 0.0) + client.get_integer_assignment("experiment-key-1", "user-1", {}, 1) + client.get_string_assignment("experiment-key-1", "user-1", {}, "default value") + client.get_json_assignment("experiment-key-1", "user-1", {}, {"hello": "world"}) def test_client_has_flags(): @@ -241,10 +244,10 @@ def get_assignments(test_case, get_assignment_fn): assignments = [] for subject in test_case.get("subjects", []): variation = get_assignment_fn( - subject["subjectKey"], test_case["flag"], - test_case["defaultValue"], + subject["subjectKey"], subject["subjectAttributes"], + test_case["defaultValue"], ) assignments.append((subject, variation)) return assignments From 6f35c5ae1e8d24e5db917e37d8ba95e3d9892467 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Mon, 22 Apr 2024 10:50:03 -0700 Subject: [PATCH 37/40] added semver tests --- test/rules_test.py | 99 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 98 insertions(+), 1 deletion(-) diff --git a/test/rules_test.py b/test/rules_test.py index 4c67079..7741b48 100644 --- a/test/rules_test.py +++ b/test/rules_test.py @@ -208,17 +208,114 @@ def test_evaluate_condition_lt(): ) -def test_evaluate_condition_semver(): +def test_evaluate_condition_semver_gte(): assert evaluate_condition( Condition(operator=OperatorType.GTE, value="1.0.0", attribute="version"), {"version": "1.0.1"}, ) + + assert evaluate_condition( + Condition(operator=OperatorType.GTE, value="1.0.0", attribute="version"), + {"version": "1.0.0"}, + ) + + assert not evaluate_condition( + Condition(operator=OperatorType.GTE, value="1.10.0", attribute="version"), + {"version": "1.2.0"}, + ) + + assert evaluate_condition( + Condition(operator=OperatorType.GTE, value="1.5.0", attribute="version"), + {"version": "1.13.0"}, + ) + assert not evaluate_condition( Condition(operator=OperatorType.GTE, value="1.0.0", attribute="version"), {"version": "0.9.9"}, ) +def test_evaluate_condition_semver_gt(): + assert evaluate_condition( + Condition(operator=OperatorType.GT, value="1.0.0", attribute="version"), + {"version": "1.0.1"}, + ) + + assert not evaluate_condition( + Condition(operator=OperatorType.GT, value="1.0.0", attribute="version"), + {"version": "1.0.0"}, + ) + + assert not evaluate_condition( + Condition(operator=OperatorType.GT, value="1.10.0", attribute="version"), + {"version": "1.2.903"}, + ) + + assert evaluate_condition( + Condition(operator=OperatorType.GT, value="1.5.0", attribute="version"), + {"version": "1.13.0"}, + ) + + assert not evaluate_condition( + Condition(operator=OperatorType.GT, value="1.0.0", attribute="version"), + {"version": "0.9.9"}, + ) + + +def test_evaluate_condition_semver_lte(): + assert not evaluate_condition( + Condition(operator=OperatorType.LTE, value="1.0.0", attribute="version"), + {"version": "1.0.1"}, + ) + + assert evaluate_condition( + Condition(operator=OperatorType.LTE, value="1.0.0", attribute="version"), + {"version": "1.0.0"}, + ) + + assert evaluate_condition( + Condition(operator=OperatorType.LTE, value="1.10.0", attribute="version"), + {"version": "1.2.0"}, + ) + + assert not evaluate_condition( + Condition(operator=OperatorType.LTE, value="1.5.0", attribute="version"), + {"version": "1.13.0"}, + ) + + assert evaluate_condition( + Condition(operator=OperatorType.LTE, value="1.0.0", attribute="version"), + {"version": "0.9.9"}, + ) + + +def test_evaluate_condition_semver_lt(): + assert not evaluate_condition( + Condition(operator=OperatorType.LT, value="1.0.0", attribute="version"), + {"version": "1.0.1"}, + ) + + assert not evaluate_condition( + Condition(operator=OperatorType.LT, value="1.0.0", attribute="version"), + {"version": "1.0.0"}, + ) + + assert evaluate_condition( + Condition(operator=OperatorType.LT, value="1.10.0", attribute="version"), + {"version": "1.2.0"}, + ) + + assert not evaluate_condition( + Condition(operator=OperatorType.LT, value="1.5.0", attribute="version"), + {"version": "1.13.0"}, + ) + + assert evaluate_condition( + Condition(operator=OperatorType.LT, value="1.0.0", attribute="version"), + {"version": "0.9.9"}, + ) + + def test_evaluate_condition_one_of_int(): one_of_condition_int = Condition( operator=OperatorType.ONE_OF, value=[10, 20, 30], attribute="number" From 82bfa593825160003e6785755904ada178788cda Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Mon, 22 Apr 2024 10:50:10 -0700 Subject: [PATCH 38/40] return noneresult more often --- eppo_client/client.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/eppo_client/client.py b/eppo_client/client.py index d5904e2..20ad0ad 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -12,7 +12,7 @@ from eppo_client.sharders import MD5Sharder from eppo_client.types import SubjectAttributes, ValueType from eppo_client.validation import validate_not_blank -from eppo_client.eval import FlagEvaluation, Evaluator +from eppo_client.eval import FlagEvaluation, Evaluator, none_result from eppo_client.version import __version__ @@ -169,7 +169,7 @@ def get_assignment_detail( logger.warning( "[Eppo SDK] No assigned variation. Flag not found: " + flag_key ) - return None + return none_result(flag_key, None, subject_key, subject_attributes) if not check_type_match(expected_variation_type, flag.variation_type): raise TypeError( @@ -181,7 +181,7 @@ def get_assignment_detail( logger.info( "[Eppo SDK] No assigned variation. Flag is disabled: " + flag_key ) - return None + return none_result(flag_key, None, subject_key, subject_attributes) result = self.__evaluator.evaluate_flag(flag, subject_key, subject_attributes) @@ -192,7 +192,9 @@ def get_assignment_detail( "[Eppo SDK] Variation value does not have the correct type for the flag: " f"{flag_key} and variation key {result.variation.key}" ) - return None + return none_result( + flag_key, flag.variation_type, subject_key, subject_attributes + ) assignment_event = { **(result.extra_logging if result else {}), From 48640855e04ddf898a8c2934d7b62e9f54621023 Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Mon, 22 Apr 2024 10:52:59 -0700 Subject: [PATCH 39/40] add sharders test --- test/sharders_test.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 test/sharders_test.py diff --git a/test/sharders_test.py b/test/sharders_test.py new file mode 100644 index 0000000..55b0c89 --- /dev/null +++ b/test/sharders_test.py @@ -0,0 +1,30 @@ +from eppo_client.sharders import MD5Sharder, DeterministicSharder + + +def test_md5_sharder(): + sharder = MD5Sharder() + inputs = [ + ("test-input", 5619), + ("alice", 3170), + ("bob", 7420), + ("charlie", 7497), + ] + total_shards = 10000 + for input, expected_shard in inputs: + assert sharder.get_shard(input, total_shards) == expected_shard + + +def test_deterministic_sharder_present(): + lookup = {"test-input": 5} + sharder = DeterministicSharder(lookup) + input = "test-input" + total_shards = 10 # totalShards is ignored in DeterministicSharder + assert sharder.get_shard(input, total_shards) == 5 + + +def test_deterministic_sharder_absent(): + lookup = {"some-other-input": 7} + sharder = DeterministicSharder(lookup) + input = "test-input-not-in-lookup" + total_shards = 10 # totalShards is ignored in DeterministicSharder + assert sharder.get_shard(input, total_shards) == 0 From 67c439f5530caf4e71386d26e289ea1909fbcdfc Mon Sep 17 00:00:00 2001 From: Sven Schmit Date: Mon, 22 Apr 2024 13:03:59 -0700 Subject: [PATCH 40/40] :broom: --- eppo_client/client.py | 23 ++++++++++++++--------- eppo_client/models.py | 1 - eppo_client/types.py | 2 +- test/client_test.py | 4 +++- 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/eppo_client/client.py b/eppo_client/client.py index 20ad0ad..2200bc6 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -107,24 +107,25 @@ def get_json_assignment( subject_attributes: SubjectAttributes, default: Dict[Any, Any], ) -> Dict[Any, Any]: - variation_json_string = self.get_assignment_variation( + json_value = self.get_assignment_variation( flag_key, subject_key, subject_attributes, None, VariationType.JSON, ) - if variation_json_string is None: + if json_value is None: return default - return json.loads(variation_json_string) + + return json.loads(json_value) def get_assignment_variation( self, flag_key: str, subject_key: str, subject_attributes: SubjectAttributes, - default: Optional[ValueType] = None, - expected_variation_type: Optional[VariationType] = None, + default: Optional[ValueType], + expected_variation_type: VariationType, ): try: result = self.get_assignment_detail( @@ -147,8 +148,8 @@ def get_assignment_detail( flag_key: str, subject_key: str, subject_attributes: SubjectAttributes, - expected_variation_type: Optional[VariationType] = None, - ) -> Optional[FlagEvaluation]: + expected_variation_type: VariationType, + ) -> FlagEvaluation: """Maps a subject to a variation for a given flag Returns None if the subject is not allocated in the flag @@ -169,7 +170,9 @@ def get_assignment_detail( logger.warning( "[Eppo SDK] No assigned variation. Flag not found: " + flag_key ) - return none_result(flag_key, None, subject_key, subject_attributes) + return none_result( + flag_key, expected_variation_type, subject_key, subject_attributes + ) if not check_type_match(expected_variation_type, flag.variation_type): raise TypeError( @@ -181,7 +184,9 @@ def get_assignment_detail( logger.info( "[Eppo SDK] No assigned variation. Flag is disabled: " + flag_key ) - return none_result(flag_key, None, subject_key, subject_attributes) + return none_result( + flag_key, expected_variation_type, subject_key, subject_attributes + ) result = self.__evaluator.evaluate_flag(flag, subject_key, subject_attributes) diff --git a/eppo_client/models.py b/eppo_client/models.py index a88a122..601bc86 100644 --- a/eppo_client/models.py +++ b/eppo_client/models.py @@ -1,6 +1,5 @@ from datetime import datetime from enum import Enum - from typing import Dict, List, Optional from eppo_client.base_model import SdkBaseModel diff --git a/eppo_client/types.py b/eppo_client/types.py index 26676b9..dcc4a76 100644 --- a/eppo_client/types.py +++ b/eppo_client/types.py @@ -1,4 +1,4 @@ -from typing import List, Union, Dict +from typing import Dict, List, Union ValueType = Union[str, int, float, bool] AttributeType = Union[str, int, float, bool] diff --git a/test/client_test.py b/test/client_test.py index e34ea04..a260c88 100644 --- a/test/client_test.py +++ b/test/client_test.py @@ -176,7 +176,9 @@ def test_graceful_mode_on(get_assignment_detail, mock_config_requestor): ) assert ( - client.get_assignment_variation("experiment-key-1", "user-1", {}, "default") + client.get_assignment_variation( + "experiment-key-1", "user-1", {}, "default", VariationType.STRING + ) == "default" ) assert client.get_boolean_assignment("experiment-key-1", "user-1", {}, default=True)