From e64533ced9307384709511668d9269b62032955d Mon Sep 17 00:00:00 2001 From: cosmos-explorer <88151306+cosmos-explorer@users.noreply.github.com> Date: Wed, 4 Nov 2026 11:02:10 +0100 Subject: [PATCH] refactor: remove SDK experiment eligibility reporting --- fbclient/common_types.py | 8 +-- fbclient/evaluator.py | 28 +--------- fbclient/event_types.py | 1 - tests/fbclient_test_data.json | 97 ++++++++-------------------------- tests/test_insights_enabled.py | 13 +++-- 5 files changed, 35 insertions(+), 112 deletions(-) diff --git a/fbclient/common_types.py b/fbclient/common_types.py index 921881e..cb5ed6c 100644 --- a/fbclient/common_types.py +++ b/fbclient/common_types.py @@ -285,7 +285,6 @@ def __init__(self, id: str, value: Optional[str], reason: str, - is_send_to_expt: bool = False, key_name: Optional[str] = None, name: Optional[str] = None, flag_type: Optional[str] = 'string'): @@ -293,14 +292,13 @@ def __init__(self, self.__id = id self.__value = value self.__reason = reason - self.__is_send_to_expt = is_send_to_expt self.__key_name = key_name self.__name = name self.__flag_type = flag_type @staticmethod def error(default_value: Optional[str], reason: str, key_name: Optional[str] = None, flag_type: Optional[str] = 'string'): - return _EvalResult(__NO_VARIATION__, default_value, reason, False, key_name if key_name else __FLAG_KEY_UNKNOWN__, __FLAG_NAME_UNKNOWN__, flag_type) + return _EvalResult(__NO_VARIATION__, default_value, reason, key_name if key_name else __FLAG_KEY_UNKNOWN__, __FLAG_NAME_UNKNOWN__, flag_type) @property def id(self) -> str: @@ -314,10 +312,6 @@ def value(self) -> Optional[str]: def reason(self) -> str: return self.__reason - @property - def is_send_to_expt(self) -> bool: - return self.__is_send_to_expt - @property def key_name(self) -> Optional[str]: return self.__key_name diff --git a/fbclient/evaluator.py b/fbclient/evaluator.py index ae9ea6f..f47bb02 100644 --- a/fbclient/evaluator.py +++ b/fbclient/evaluator.py @@ -66,8 +66,6 @@ __NOT_IN_SEGMENT_CLAUSE__ = 'User is not in segment' -__EXPT_KEY_PREFIX__ = "expt" - class Evaluator: def __init__(self, @@ -96,7 +94,7 @@ def _match_user_variation(self, flag: dict, user: FBUser, fb_event: Optional[FBE if er is not None: log.info('FB Python SDK: User %s, Feature Flag %s, Flag Value %s' % (user.get('KeyId'), er.key_name, er.value)) # Match the server default: only an explicit boolean false - # disables insights; evaluation and experiment eligibility stay unchanged. + # disables insights; evaluation stays unchanged. if fb_event is not None and flag.get('insightsEnabled') is not False: fb_event.add(FlagEventVariation(er)) @@ -106,7 +104,6 @@ def _match_feature_flag_disabled_user_variation(self, flag: dict, user: FBUser) return _EvalResult(flag['disabledVariationId'], flag['variationMap'][flag['disabledVariationId']], REASON_FLAG_OFF, - False, flag['key'], flag['name'], flag['variationType']) @@ -119,7 +116,6 @@ def _match_targeted_user_variation(self, flag: dict, user: FBUser) -> Optional[_ return _EvalResult(target['variationId'], flag['variationMap'][target['variationId']], REASON_TARGET_MATCH, - flag['exptIncludeAllTargets'], flag['key'], flag['name'], flag['variationType']) @@ -269,36 +265,14 @@ def _get_rollout_variation_option(self, key_name: str, name: str) -> Optional[_EvalResult]: - def is_send_to_expt(dispatch_key: str, - rollout: dict, - expt_include_all_targets: bool, - rule_inclued_in_expt: bool) -> bool: - if expt_include_all_targets: - return True - if rule_inclued_in_expt: - send_to_expt_percentage = rollout['exptRollout'] - splitting_percentage = rollout['rollout'][1] - rollout['rollout'][0] - - if send_to_expt_percentage == 0 or splitting_percentage == 0: - return False - - upper_bound = send_to_expt_percentage / splitting_percentage - if upper_bound > 1: - upper_bound = 1 - new_dispatch_key = "".join((__EXPT_KEY_PREFIX__, dispatch_key)) - return VariationSplittingAlgorithm(new_dispatch_key, [0, upper_bound]).is_key_belongs_to_percentage() - return False - dispatch_key = rollout_variations.get('dispatchKey') dispatch_key = dispatch_key if dispatch_key else 'keyid' dispatch_key_value = "".join((flag['key'], user.get(dispatch_key, ""))) # type: ignore for rollout in rollout_variations['variations']: if VariationSplittingAlgorithm(dispatch_key_value, rollout['rollout']).is_key_belongs_to_percentage(): # type: ignore - send_to_expt = is_send_to_expt(dispatch_key_value, rollout, flag['exptIncludeAllTargets'], rollout_variations['includedInExpt']) # type: ignore return _EvalResult(rollout['id'], flag['variationMap'][rollout['id']], reason, - send_to_expt, key_name, name, flag['variationType']) diff --git a/fbclient/event_types.py b/fbclient/event_types.py index 37611dc..50f332a 100644 --- a/fbclient/event_types.py +++ b/fbclient/event_types.py @@ -121,7 +121,6 @@ def to_json_dict(self) -> dict: arr = [] for variation in self.__variations: flag_json_dict = {'featureFlagKey': variation.variation.key_name, - 'sendToExperiment': variation.variation.is_send_to_expt, 'timestamp': variation.timestamp, 'variation': { 'id': variation.variation.id, diff --git a/tests/fbclient_test_data.json b/tests/fbclient_test_data.json index ad6366a..ec4e159 100644 --- a/tests/fbclient_test_data.json +++ b/tests/fbclient_test_data.json @@ -24,7 +24,6 @@ "id": "cf1ca836-a4ef-422e-8996-931aecd248a2", "name": "segment rule", "dispatchKey": null, - "includedInExpt": false, "conditions": [ { "property": "User is in segment", @@ -38,8 +37,7 @@ "rollout": [ 0, 1 - ], - "exptRollout": 1 + ] } ] } @@ -48,19 +46,16 @@ "disabledVariationId": "f62a3eae-e47f-481c-83eb-1b8fb3c39c9b", "fallthrough": { "dispatchKey": null, - "includedInExpt": true, "variations": [ { "id": "f62a3eae-e47f-481c-83eb-1b8fb3c39c9b", "rollout": [ 0, 1 - ], - "exptRollout": 1 + ] } ] }, - "exptIncludeAllTargets": true, "tags": [], "isArchived": false, "disabledVariation": { @@ -102,27 +97,23 @@ "disabledVariationId": "43a1b83b-18df-42a6-971c-b2518455d8f2", "fallthrough": { "dispatchKey": "name", - "includedInExpt": true, "variations": [ { "id": "18b369f8-453f-46d7-88cc-fe41d29ca6e3", "rollout": [ 0, 0.5 - ], - "exptRollout": 1 + ] }, { "id": "43a1b83b-18df-42a6-971c-b2518455d8f2", "rollout": [ 0.5, 1 - ], - "exptRollout": 1 + ] } ] }, - "exptIncludeAllTargets": true, "tags": [], "isArchived": false, "disabledVariation": { @@ -164,7 +155,6 @@ "id": "41402f94-64c9-4aca-9016-58a019a917c9", "name": "USA", "dispatchKey": null, - "includedInExpt": false, "conditions": [ { "property": "country", @@ -178,8 +168,7 @@ "rollout": [ 0, 1 - ], - "exptRollout": 1 + ] } ] }, @@ -187,7 +176,6 @@ "id": "5a92deed-68db-4dc5-a8d1-721a275180c5", "name": "FRANCE", "dispatchKey": null, - "includedInExpt": false, "conditions": [ { "property": "country", @@ -201,8 +189,7 @@ "rollout": [ 0, 1 - ], - "exptRollout": 1 + ] } ] }, @@ -210,7 +197,6 @@ "id": "5e0a88cd-7117-4d1c-972b-7dc9e747dddc", "name": "CHINA", "dispatchKey": null, - "includedInExpt": false, "conditions": [ { "property": "country", @@ -224,8 +210,7 @@ "rollout": [ 0, 1 - ], - "exptRollout": 1 + ] } ] } @@ -234,19 +219,16 @@ "disabledVariationId": "d1192640-dd82-4dbe-823e-88d7794b94ee", "fallthrough": { "dispatchKey": null, - "includedInExpt": true, "variations": [ { "id": "d1192640-dd82-4dbe-823e-88d7794b94ee", "rollout": [ 0, 1 - ], - "exptRollout": 1 + ] } ] }, - "exptIncludeAllTargets": true, "tags": [], "isArchived": false, "disabledVariation": { @@ -284,7 +266,6 @@ "id": "ed5843e7-0f38-4d61-9945-b7dad6694601", "name": "Email", "dispatchKey": null, - "includedInExpt": false, "conditions": [ { "property": "keyId", @@ -298,8 +279,7 @@ "rollout": [ 0, 1 - ], - "exptRollout": 1 + ] } ] }, @@ -307,7 +287,6 @@ "id": "08187230-7ad4-43c2-b830-b2d4269cecb1", "name": "CN Phone Number", "dispatchKey": null, - "includedInExpt": false, "conditions": [ { "property": "keyId", @@ -321,8 +300,7 @@ "rollout": [ 0, 1 - ], - "exptRollout": 1 + ] } ] }, @@ -330,7 +308,6 @@ "id": "3fea476d-fb36-4bd1-8615-1b607926f314", "name": "FR Phone Number", "dispatchKey": null, - "includedInExpt": false, "conditions": [ { "property": "keyId", @@ -344,8 +321,7 @@ "rollout": [ 0, 1 - ], - "exptRollout": 1 + ] } ] } @@ -354,19 +330,16 @@ "disabledVariationId": "c4ed60ff-d195-415d-a657-3243286673b7", "fallthrough": { "dispatchKey": null, - "includedInExpt": true, "variations": [ { "id": "c4ed60ff-d195-415d-a657-3243286673b7", "rollout": [ 0, 1 - ], - "exptRollout": 1 + ] } ] }, - "exptIncludeAllTargets": true, "tags": [], "isArchived": false, "disabledVariation": { @@ -400,27 +373,23 @@ "disabledVariationId": "0127cc87-99e2-4c46-98d9-5974a4fa1de1", "fallthrough": { "dispatchKey": "name", - "includedInExpt": true, "variations": [ { "id": "255ee1b9-0290-4ffe-98f8-06c4b66f3608", "rollout": [ 0, 0.25 - ], - "exptRollout": 1 + ] }, { "id": "0127cc87-99e2-4c46-98d9-5974a4fa1de1", "rollout": [ 0.25, 1 - ], - "exptRollout": 1 + ] } ] }, - "exptIncludeAllTargets": true, "tags": [], "isArchived": false, "disabledVariation": { @@ -454,19 +423,16 @@ "disabledVariationId": "628ab219-b397-428b-84ce-a1734046c867", "fallthrough": { "dispatchKey": null, - "includedInExpt": true, "variations": [ { "id": "ab9488b1-c6d0-444d-ad29-8738f9a5f1a3", "rollout": [ 0, 1 - ], - "exptRollout": 1 + ] } ] }, - "exptIncludeAllTargets": true, "tags": [], "isArchived": false, "disabledVariation": { @@ -535,7 +501,6 @@ "id": "d999869e-a287-414f-a13e-04accd03dc61", "name": "True Rule", "dispatchKey": null, - "includedInExpt": false, "conditions": [ { "property": "graduated", @@ -549,8 +514,7 @@ "rollout": [ 0, 1 - ], - "exptRollout": 1 + ] } ] }, @@ -558,7 +522,6 @@ "id": "a8da5927-63ed-412b-8728-f950cc7f785e", "name": "Equal Rule", "dispatchKey": null, - "includedInExpt": false, "conditions": [ { "property": "country", @@ -572,8 +535,7 @@ "rollout": [ 0, 1 - ], - "exptRollout": 1 + ] } ] }, @@ -581,7 +543,6 @@ "id": "a1a217c0-d91f-4ea2-b45f-993444ef743c", "name": "Than Rule", "dispatchKey": null, - "includedInExpt": false, "conditions": [ { "property": "salary", @@ -600,8 +561,7 @@ "rollout": [ 0, 1 - ], - "exptRollout": 1 + ] } ] }, @@ -609,7 +569,6 @@ "id": "bfd4fca9-7131-4bcc-b7c2-c49c3146387c", "name": "Contain Rule", "dispatchKey": null, - "includedInExpt": false, "conditions": [ { "property": "email", @@ -623,8 +582,7 @@ "rollout": [ 0, 1 - ], - "exptRollout": 1 + ] } ] }, @@ -632,7 +590,6 @@ "id": "9abbd28c-088a-4d9c-8b68-7d1f4494b18f", "name": "OneOf Rule", "dispatchKey": null, - "includedInExpt": false, "conditions": [ { "property": "major", @@ -646,8 +603,7 @@ "rollout": [ 0, 1 - ], - "exptRollout": 1 + ] } ] }, @@ -655,7 +611,6 @@ "id": "d9cd384c-6282-482d-a43d-5d4461068e18", "name": "StartEnd Rule", "dispatchKey": null, - "includedInExpt": false, "conditions": [ { "property": "name", @@ -674,8 +629,7 @@ "rollout": [ 0, 1 - ], - "exptRollout": 1 + ] } ] }, @@ -683,7 +637,6 @@ "id": "ecdc09b0-7aa0-485d-8498-990c8589c95c", "name": "MatchRegex Rule", "dispatchKey": null, - "includedInExpt": false, "conditions": [ { "property": "phone", @@ -697,8 +650,7 @@ "rollout": [ 0, 1 - ], - "exptRollout": 1 + ] } ] } @@ -707,19 +659,16 @@ "disabledVariationId": "ac21c2fc-eec4-4b49-ad80-cc525720166b", "fallthrough": { "dispatchKey": null, - "includedInExpt": true, "variations": [ { "id": "ac21c2fc-eec4-4b49-ad80-cc525720166b", "rollout": [ 0, 1 - ], - "exptRollout": 1 + ] } ] }, - "exptIncludeAllTargets": true, "tags": [], "isArchived": false, "disabledVariation": { diff --git a/tests/test_insights_enabled.py b/tests/test_insights_enabled.py index c4bb9fc..ae24931 100644 --- a/tests/test_insights_enabled.py +++ b/tests/test_insights_enabled.py @@ -98,7 +98,7 @@ def test_external_json_preserves_insights_setting(setting): ('ff-evaluation-test', {'key': 'rule-user', 'graduated': 'true'}), ('ff-evaluation-test', {'key': 'fallthrough-user'}), ]) -def test_insights_does_not_change_evaluation_or_experiment_eligibility(client, key, user): +def test_insights_does_not_change_evaluation(client, key, user): sync(client, payload(True)) flag = client._get_flag_internal(key) user = FBUser.from_dict(dict(user, name='Test User')) @@ -107,9 +107,16 @@ def test_insights_does_not_change_evaluation_or_experiment_eligibility(client, k flag = dict(flag, insightsEnabled=False) disabled_event = FlagEvent(user) after = client._evaluator.evaluate(flag, user, disabled_event) - assert (after.id, after.value, after.reason, after.is_send_to_expt) == ( - before.id, before.value, before.reason, before.is_send_to_expt) + assert (after.id, after.value, after.reason) == ( + before.id, before.value, before.reason) assert enabled_event.is_send_event + reported = enabled_event.to_json_dict()['variations'][0] + assert reported == { + 'featureFlagKey': key, + 'timestamp': reported['timestamp'], + 'variation': {'id': before.id, 'value': before.value, 'reason': before.reason}, + } + assert isinstance(reported['timestamp'], int) assert not disabled_event.is_send_event