From 969be5f1d9b12be398eefae85d1c012db2675c28 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Mon, 24 Mar 2025 20:31:23 +0600 Subject: [PATCH 1/3] experiment_id and variation_id added to payloads --- optimizely/optimizely.py | 39 +++++++++++++++++++++++++++++++----- optimizely/project_config.py | 28 ++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 1b25bec6..4c27b814 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -340,7 +340,9 @@ def _get_feature_variable_for_type( user_context = OptimizelyUserContext(self, self.logger, user_id, attributes, False) decision, _ = self.decision_service.get_variation_for_feature(project_config, feature_flag, user_context) - + experiment_id = decision.experiment.id if decision.experiment else None + variation_id = decision.variation.id if decision.variation else None + if decision.variation: feature_enabled = decision.variation.featureEnabled @@ -386,6 +388,8 @@ def _get_feature_variable_for_type( 'variable_value': actual_value, 'variable_type': variable_type, 'source_info': source_info, + 'experiment_id': experiment_id, + 'variation_id': variation_id }, ) return actual_value @@ -427,7 +431,9 @@ def _get_all_feature_variables_for_type( user_context = OptimizelyUserContext(self, self.logger, user_id, attributes, False) decision, _ = self.decision_service.get_variation_for_feature(project_config, feature_flag, user_context) - + experiment_id = decision.experiment.id if decision.experiment else None + variation_id = decision.variation.id if decision.variation else None + if decision.variation: feature_enabled = decision.variation.featureEnabled @@ -480,6 +486,8 @@ def _get_all_feature_variables_for_type( 'variable_values': all_variables, 'source': decision.source, 'source_info': source_info, + 'experiment_id': experiment_id, + 'variation_id': variation_id }, ) return all_variables @@ -646,13 +654,21 @@ def get_variation( decision_notification_type = enums.DecisionNotificationTypes.FEATURE_TEST else: decision_notification_type = enums.DecisionNotificationTypes.AB_TEST - + + experiment_id = experiment.id if experiment else None + variation_id = variation.id if variation else None + self.notification_center.send_notifications( enums.NotificationTypes.DECISION, decision_notification_type, user_id, attributes or {}, - {'experiment_key': experiment_key, 'variation_key': variation_key}, + { + 'experiment_key': experiment_key, + 'variation_key': variation_key, + 'experiment_id': experiment_id, + 'variation_id': variation_id + }, ) return variation_key @@ -738,6 +754,8 @@ def is_feature_enabled(self, feature_key: str, user_id: str, attributes: Optiona 'feature_enabled': feature_enabled, 'source': decision.source, 'source_info': source_info, + 'experiment_id': decision.experiment.id, + 'variation_id': decision.variation.id }, ) @@ -1202,6 +1220,15 @@ def _create_optimizely_decision( if flag_decision is not None and flag_decision.variation is not None else None ) + + rollout_id = feature_flag.rolloutId if decision_source == DecisionSources.ROLLOUT else None + experiment_id = project_config.get_experiment_id_by_key_or_rollout_id(rule_key, rollout_id) + variation_id = None + if variation_key: + variation = project_config.get_variation_from_key_by_experiment_id(experiment_id, variation_key) + if variation: + variation_id = variation.id + # Send notification self.notification_center.send_notifications( enums.NotificationTypes.DECISION, @@ -1215,7 +1242,9 @@ def _create_optimizely_decision( 'variation_key': variation_key, 'rule_key': rule_key, 'reasons': decision_reasons if should_include_reasons else [], - 'decision_event_dispatched': decision_event_dispatched + 'decision_event_dispatched': decision_event_dispatched, + 'experiment_id': experiment_id, + 'variation_id': variation_id }, ) diff --git a/optimizely/project_config.py b/optimizely/project_config.py index adfeee41..b815d189 100644 --- a/optimizely/project_config.py +++ b/optimizely/project_config.py @@ -716,3 +716,31 @@ def get_flag_variation( return variation return None + + def get_experiment_id_by_key_or_rollout_id(self, key: str, rollout_id: Optional[str] = None) -> Optional[str]: + """ + Retrieves the experiment ID associated with a given rule key or a specific rollout. + + Args: + key: The key associated with the experiment rule. + rollout_id: The ID of the rollout to search if the key is not found. + + Returns: + Optional[str]: The experiment ID if found, otherwise None. + """ + # Try getting the experiment from experiment_key_map first + if key: + experiment = self.get_experiment_from_key(key) + if experiment: + return experiment.id + + # If key is not found in experiment_key_map, check a specific rollout (if provided) + if rollout_id: + rollout = self.get_rollout_from_id(rollout_id) + if rollout: + for experiment in rollout.experiments: + experiment = entities.Experiment(**experiment) + if experiment.key == key: + return experiment.id + + return None From 60dba53c18a42a8c5c7ecabfd673ca35e2d77f72 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Fri, 4 Apr 2025 20:11:23 +0600 Subject: [PATCH 2/3] optimizely/optimizely.py -> Removed experiment_id and variation_id from legacy apis. optimizely/project_config.py -> Enhanced comments for clarity. tests/test_user_context.py -> Updated test assertions for experiments. --- optimizely/optimizely.py | 37 +++++++------------ optimizely/project_config.py | 10 +++--- tests/test_user_context.py | 70 ++++++++++++++++++++++++------------ 3 files changed, 65 insertions(+), 52 deletions(-) diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 4c27b814..c7370573 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -340,9 +340,7 @@ def _get_feature_variable_for_type( user_context = OptimizelyUserContext(self, self.logger, user_id, attributes, False) decision, _ = self.decision_service.get_variation_for_feature(project_config, feature_flag, user_context) - experiment_id = decision.experiment.id if decision.experiment else None - variation_id = decision.variation.id if decision.variation else None - + if decision.variation: feature_enabled = decision.variation.featureEnabled @@ -388,8 +386,6 @@ def _get_feature_variable_for_type( 'variable_value': actual_value, 'variable_type': variable_type, 'source_info': source_info, - 'experiment_id': experiment_id, - 'variation_id': variation_id }, ) return actual_value @@ -431,9 +427,7 @@ def _get_all_feature_variables_for_type( user_context = OptimizelyUserContext(self, self.logger, user_id, attributes, False) decision, _ = self.decision_service.get_variation_for_feature(project_config, feature_flag, user_context) - experiment_id = decision.experiment.id if decision.experiment else None - variation_id = decision.variation.id if decision.variation else None - + if decision.variation: feature_enabled = decision.variation.featureEnabled @@ -486,8 +480,6 @@ def _get_all_feature_variables_for_type( 'variable_values': all_variables, 'source': decision.source, 'source_info': source_info, - 'experiment_id': experiment_id, - 'variation_id': variation_id }, ) return all_variables @@ -654,10 +646,7 @@ def get_variation( decision_notification_type = enums.DecisionNotificationTypes.FEATURE_TEST else: decision_notification_type = enums.DecisionNotificationTypes.AB_TEST - - experiment_id = experiment.id if experiment else None - variation_id = variation.id if variation else None - + self.notification_center.send_notifications( enums.NotificationTypes.DECISION, decision_notification_type, @@ -666,8 +655,6 @@ def get_variation( { 'experiment_key': experiment_key, 'variation_key': variation_key, - 'experiment_id': experiment_id, - 'variation_id': variation_id }, ) @@ -754,8 +741,6 @@ def is_feature_enabled(self, feature_key: str, user_id: str, attributes: Optiona 'feature_enabled': feature_enabled, 'source': decision.source, 'source_info': source_info, - 'experiment_id': decision.experiment.id, - 'variation_id': decision.variation.id }, ) @@ -1220,15 +1205,19 @@ def _create_optimizely_decision( if flag_decision is not None and flag_decision.variation is not None else None ) - - rollout_id = feature_flag.rolloutId if decision_source == DecisionSources.ROLLOUT else None - experiment_id = project_config.get_experiment_id_by_key_or_rollout_id(rule_key, rollout_id) + + rollout_id = None + if decision_source == DecisionSources.ROLLOUT and feature_flag is not None: + rollout_id = feature_flag.rolloutId + experiment_id = None + if rule_key is not None: + experiment_id = project_config.get_experiment_id_by_key_or_rollout_id(rule_key, rollout_id) variation_id = None - if variation_key: + if experiment_id and variation_key: variation = project_config.get_variation_from_key_by_experiment_id(experiment_id, variation_key) if variation: - variation_id = variation.id - + variation_id = variation.id + # Send notification self.notification_center.send_notifications( enums.NotificationTypes.DECISION, diff --git a/optimizely/project_config.py b/optimizely/project_config.py index b815d189..35b17ad1 100644 --- a/optimizely/project_config.py +++ b/optimizely/project_config.py @@ -722,8 +722,8 @@ def get_experiment_id_by_key_or_rollout_id(self, key: str, rollout_id: Optional[ Retrieves the experiment ID associated with a given rule key or a specific rollout. Args: - key: The key associated with the experiment rule. - rollout_id: The ID of the rollout to search if the key is not found. + key: The key associated with the experiment rule. It can be experiment key or rule key. + rollout_id: The ID of the rollout to be searched if the key is not found in the experiment key map. Returns: Optional[str]: The experiment ID if found, otherwise None. @@ -738,9 +738,9 @@ def get_experiment_id_by_key_or_rollout_id(self, key: str, rollout_id: Optional[ if rollout_id: rollout = self.get_rollout_from_id(rollout_id) if rollout: - for experiment in rollout.experiments: - experiment = entities.Experiment(**experiment) + for experiment_data in rollout.experiments: + experiment = entities.Experiment(**experiment_data) if experiment.key == key: - return experiment.id + return experiment.id return None diff --git a/tests/test_user_context.py b/tests/test_user_context.py index 0c35e230..6705e414 100644 --- a/tests/test_user_context.py +++ b/tests/test_user_context.py @@ -283,6 +283,8 @@ def test_decide__feature_test(self): 'reasons': expected.reasons, 'decision_event_dispatched': True, 'variables': expected.variables, + 'experiment_id': mock_experiment.id, + 'variation_id': mock_variation.id }, ) @@ -391,6 +393,24 @@ def test_decide_feature_rollout(self): self.compare_opt_decisions(expected, actual) + # assert event count + self.assertEqual(1, mock_send_event.call_count) + + # assert event payload + expected_experiment = project_config.get_experiment_from_key(expected.rule_key) + expected_var = project_config.get_variation_from_key(expected.rule_key, expected.variation_key) + mock_send_event.assert_called_with( + project_config, + expected_experiment, + expected_var, + expected.flag_key, + expected.rule_key, + 'rollout', + expected.enabled, + 'test_user', + user_attributes + ) + # assert notification count self.assertEqual(1, mock_broadcast_decision.call_count) @@ -408,27 +428,11 @@ def test_decide_feature_rollout(self): 'reasons': expected.reasons, 'decision_event_dispatched': True, 'variables': expected.variables, + 'experiment_id': expected_experiment.id, + 'variation_id': expected_var.id }, ) - # assert event count - self.assertEqual(1, mock_send_event.call_count) - - # assert event payload - expected_experiment = project_config.get_experiment_from_key(expected.rule_key) - expected_var = project_config.get_variation_from_key(expected.rule_key, expected.variation_key) - mock_send_event.assert_called_with( - project_config, - expected_experiment, - expected_var, - expected.flag_key, - expected.rule_key, - 'rollout', - expected.enabled, - 'test_user', - user_attributes - ) - def test_decide_feature_rollout__send_flag_decision_false(self): opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) project_config = opt_obj.config_manager.get_config() @@ -467,6 +471,8 @@ def test_decide_feature_rollout__send_flag_decision_false(self): self.assertEqual(1, mock_broadcast_decision.call_count) # assert notification + expected_experiment = project_config.get_experiment_from_key(expected.rule_key) + expected_var = project_config.get_variation_from_key(expected.rule_key, expected.variation_key) mock_broadcast_decision.assert_called_with( enums.NotificationTypes.DECISION, 'flag', @@ -480,6 +486,8 @@ def test_decide_feature_rollout__send_flag_decision_false(self): 'reasons': expected.reasons, 'decision_event_dispatched': False, 'variables': expected.variables, + 'experiment_id': expected_experiment.id, + 'variation_id': expected_var.id }, ) @@ -549,7 +557,9 @@ def test_decide_feature_null_variation(self): 'reasons': expected.reasons, 'decision_event_dispatched': True, 'variables': expected.variables, - }, + 'experiment_id': None, + 'variation_id': None + } ) # assert event count @@ -632,6 +642,8 @@ def test_decide_feature_null_variation__send_flag_decision_false(self): 'reasons': expected.reasons, 'decision_event_dispatched': False, 'variables': expected.variables, + 'experiment_id': None, + 'variation_id': None }, ) @@ -701,6 +713,8 @@ def test_decide__option__disable_decision_event(self): 'reasons': expected.reasons, 'decision_event_dispatched': False, 'variables': expected.variables, + 'experiment_id': mock_experiment.id, + 'variation_id': mock_variation.id, }, ) @@ -773,6 +787,8 @@ def test_decide__default_option__disable_decision_event(self): 'reasons': expected.reasons, 'decision_event_dispatched': False, 'variables': expected.variables, + 'experiment_id': mock_experiment.id, + 'variation_id': mock_variation.id }, ) @@ -834,6 +850,8 @@ def test_decide__option__exclude_variables(self): 'reasons': expected.reasons, 'decision_event_dispatched': True, 'variables': expected.variables, + 'experiment_id': mock_experiment.id, + 'variation_id': mock_variation.id, }, ) @@ -948,6 +966,8 @@ def test_decide__option__enabled_flags_only(self): 'reasons': expected.reasons, 'decision_event_dispatched': True, 'variables': expected.variables, + 'experiment_id': expected_experiment.id, + 'variation_id': expected_var.id, }, ) @@ -1006,7 +1026,7 @@ def test_decide__default_options__with__options(self): enabled=True, variables=expected_variables, flag_key='test_feature_in_experiment', - user_context=user_context + user_context=user_context, ) self.compare_opt_decisions(expected, actual) @@ -1025,6 +1045,8 @@ def test_decide__default_options__with__options(self): 'reasons': expected.reasons, 'decision_event_dispatched': False, 'variables': expected.variables, + 'experiment_id': mock_experiment.id, + 'variation_id': mock_variation.id }, ) @@ -1490,6 +1512,9 @@ def test_should_return_valid_decision_after_setting_and_removing_forced_decision 'User "test_user" is in variation "control" of experiment test_experiment.'] ) + expected_experiment = project_config.get_experiment_from_key(expected.rule_key) + expected_var = project_config.get_variation_from_key('test_experiment', expected.variation_key) + # assert notification count self.assertEqual(1, mock_broadcast_decision.call_count) @@ -1507,12 +1532,11 @@ def test_should_return_valid_decision_after_setting_and_removing_forced_decision 'reasons': expected.reasons, 'decision_event_dispatched': True, 'variables': expected.variables, + 'experiment_id': expected_experiment.id, + 'variation_id': expected_var.id }, ) - expected_experiment = project_config.get_experiment_from_key(expected.rule_key) - expected_var = project_config.get_variation_from_key('test_experiment', expected.variation_key) - mock_send_event.assert_called_with( project_config, expected_experiment, From 6df857a78964f6a8d1d0e04aad5c3a981659590d Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Fri, 4 Apr 2025 20:17:11 +0600 Subject: [PATCH 3/3] .flake8 -> redundant checks being performed in tests/testapp/application.py so added it to exclusions --- .flake8 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.flake8 b/.flake8 index f5990a83..0fc0cadc 100644 --- a/.flake8 +++ b/.flake8 @@ -4,5 +4,5 @@ # Line break before operand needs to be ignored for line lengths # greater than max-line-length. Best practice shows W504 ignore = E722, W504 -exclude = optimizely/lib/pymmh3.py,*virtualenv* +exclude = optimizely/lib/pymmh3.py,*virtualenv*,tests/testapp/application.py max-line-length = 120