From 18b59272a75496537ffa8c9dec832d7c59ca0744 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Tue, 12 Dec 2017 11:49:10 -0800 Subject: [PATCH] Updating find_bucket --- optimizely/bucketer.py | 12 +++++------- optimizely/decision_service.py | 19 +++++++++--------- tests/test_bucketing.py | 36 ++++++++++++---------------------- tests/test_decision_service.py | 20 ++++++++++--------- 4 files changed, 38 insertions(+), 49 deletions(-) diff --git a/optimizely/bucketer.py b/optimizely/bucketer.py index 1da1a05d..ca3d2c07 100644 --- a/optimizely/bucketer.py +++ b/optimizely/bucketer.py @@ -66,11 +66,10 @@ def _generate_bucket_value(self, bucketing_id): ratio = float(self._generate_unsigned_hash_code_32_bit(bucketing_id)) / MAX_HASH_VALUE return math.floor(ratio * MAX_TRAFFIC_VALUE) - def find_bucket(self, user_id, bucketing_id, parent_id, traffic_allocations): + def find_bucket(self, bucketing_id, parent_id, traffic_allocations): """ Determine entity based on bucket value and traffic allocations. Args: - user_id: ID for user. bucketing_id: ID to be used for bucketing the user. parent_id: ID representing group or experiment. traffic_allocations: Traffic allocations representing traffic allotted to experiments or variations. @@ -82,9 +81,8 @@ def find_bucket(self, user_id, bucketing_id, parent_id, traffic_allocations): bucketing_key = BUCKETING_ID_TEMPLATE.format(bucketing_id=bucketing_id, parent_id=parent_id) bucketing_number = self._generate_bucket_value(bucketing_key) self.config.logger.log(enums.LogLevels.DEBUG, - 'Assigned bucket %s to user "%s" with bucketing ID "%s".' % (bucketing_number, - user_id, - bucketing_id)) + 'Assigned bucket %s to user with bucketing ID "%s".' % (bucketing_number, + bucketing_id)) for traffic_allocation in traffic_allocations: current_end_of_range = traffic_allocation.get('endOfRange') @@ -115,7 +113,7 @@ def bucket(self, experiment, user_id, bucketing_id): if not group: return None - user_experiment_id = self.find_bucket(user_id, bucketing_id, experiment.groupId, group.trafficAllocation) + user_experiment_id = self.find_bucket(bucketing_id, experiment.groupId, group.trafficAllocation) if not user_experiment_id: self.config.logger.log(enums.LogLevels.INFO, 'User "%s" is in no experiment.' % user_id) return None @@ -129,7 +127,7 @@ def bucket(self, experiment, user_id, bucketing_id): (user_id, experiment.key, experiment.groupId)) # Bucket user if not in white-list and in group (if any) - variation_id = self.find_bucket(user_id, bucketing_id, experiment.id, experiment.trafficAllocation) + variation_id = self.find_bucket(bucketing_id, experiment.id, experiment.trafficAllocation) if variation_id: variation = self.config.get_variation_from_id(experiment.key, variation_id) self.config.logger.log(enums.LogLevels.INFO, 'User "%s" is in variation "%s" of experiment %s.' % diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index 0767d238..80fe8310 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -46,7 +46,7 @@ def _get_bucketing_id(user_id, attributes): attributes: Dict representing user attributes. May consist of bucketing ID to be used. Returns: - String representing bucketing ID for the user. + String representing bucketing ID for the user. Fallback to user's ID if not provided. """ attributes = attributes or {} @@ -235,29 +235,29 @@ def get_variation_for_rollout(self, rollout, user_id, attributes=None): return Decision(None, None, DECISION_SOURCE_ROLLOUT) - def get_experiment_in_group(self, group, user_id): + def get_experiment_in_group(self, group, bucketing_id): """ Determine which experiment in the group the user is bucketed into. Args: group: The group to bucket the user into. - user_id: ID of the user. + bucketing_id: ID to be used for bucketing the user. Returns: Experiment if the user is bucketed into an experiment in the specified group. None otherwise. """ - experiment_id = self.bucketer.find_bucket(user_id, group.id, group.trafficAllocation) + experiment_id = self.bucketer.find_bucket(bucketing_id, group.id, group.trafficAllocation) if experiment_id: experiment = self.config.get_experiment_from_id(experiment_id) if experiment: self.logger.log(enums.LogLevels.INFO, - 'User "%s" is in experiment %s of group %s.' % - (user_id, experiment.key, group.id)) + 'User with bucketing ID "%s" is in experiment %s of group %s.' % + (bucketing_id, experiment.key, group.id)) return experiment self.logger.log(enums.LogLevels.INFO, - 'User "%s" is not in any experiments of group %s.' % - (user_id, group.id)) + 'User with bucketing ID "%s" is not in any experiments of group %s.' % + (bucketing_id, group.id)) return None @@ -275,12 +275,13 @@ def get_variation_for_feature(self, feature, user_id, attributes=None): experiment = None variation = None + bucketing_id = self._get_bucketing_id(user_id, attributes) # First check if the feature is in a mutex group if feature.groupId: group = self.config.get_group(feature.groupId) if group: - experiment = self.get_experiment_in_group(group, user_id) + experiment = self.get_experiment_in_group(group, bucketing_id) if experiment and experiment.id in feature.experimentIds: variation = self.get_variation(experiment, user_id, attributes) diff --git a/tests/test_bucketing.py b/tests/test_bucketing.py index 5896205a..0410840a 100644 --- a/tests/test_bucketing.py +++ b/tests/test_bucketing.py @@ -160,8 +160,7 @@ def test_bucket(self): 'test_user')) self.assertEqual(2, mock_logging.call_count) - self.assertEqual(mock.call(enums.LogLevels.DEBUG, 'Assigned bucket 42 to user "test_user" ' - 'with bucketing ID "test_user".'), + self.assertEqual(mock.call(enums.LogLevels.DEBUG, 'Assigned bucket 42 to user with bucketing ID "test_user".'), mock_logging.call_args_list[0]) self.assertEqual( mock.call(enums.LogLevels.INFO, 'User "test_user" is in variation "control" of experiment test_experiment.'), @@ -176,8 +175,7 @@ def test_bucket(self): )) self.assertEqual(2, mock_logging.call_count) - self.assertEqual(mock.call(enums.LogLevels.DEBUG, 'Assigned bucket 4242 to user "test_user" ' - 'with bucketing ID "test_user".'), + self.assertEqual(mock.call(enums.LogLevels.DEBUG, 'Assigned bucket 4242 to user with bucketing ID "test_user".'), mock_logging.call_args_list[0]) self.assertEqual( mock.call(enums.LogLevels.INFO, 'User "test_user" is in no variation.'), @@ -192,8 +190,7 @@ def test_bucket(self): 'test_user', 'test_user')) self.assertEqual(2, mock_logging.call_count) - self.assertEqual(mock.call(enums.LogLevels.DEBUG, 'Assigned bucket 5042 to user "test_user" ' - 'with bucketing ID "test_user".'), + self.assertEqual(mock.call(enums.LogLevels.DEBUG, 'Assigned bucket 5042 to user with bucketing ID "test_user".'), mock_logging.call_args_list[0]) self.assertEqual( mock.call(enums.LogLevels.INFO, 'User "test_user" is in variation "variation" of experiment test_experiment.'), @@ -207,8 +204,7 @@ def test_bucket(self): 'test_user', 'test_user')) self.assertEqual(2, mock_logging.call_count) - self.assertEqual(mock.call(enums.LogLevels.DEBUG, 'Assigned bucket 424242 to user "test_user" ' - 'with bucketing ID "test_user".'), + self.assertEqual(mock.call(enums.LogLevels.DEBUG, 'Assigned bucket 424242 to user with bucketing ID "test_user".'), mock_logging.call_args_list[0]) self.assertEqual(mock.call(enums.LogLevels.INFO, 'User "test_user" is in no variation.'), mock_logging.call_args_list[1]) @@ -225,13 +221,11 @@ def test_bucket__experiment_in_group(self): 'test_user', 'test_user')) self.assertEqual(4, mock_logging.call_count) - self.assertEqual(mock.call(enums.LogLevels.DEBUG, 'Assigned bucket 42 to user "test_user" ' - 'with bucketing ID "test_user".'), + self.assertEqual(mock.call(enums.LogLevels.DEBUG, 'Assigned bucket 42 to user with bucketing ID "test_user".'), mock_logging.call_args_list[0]) self.assertEqual(mock.call(enums.LogLevels.INFO, 'User "test_user" is in experiment group_exp_1 of group 19228.'), mock_logging.call_args_list[1]) - self.assertEqual(mock.call(enums.LogLevels.DEBUG, 'Assigned bucket 4242 to user "test_user" ' - 'with bucketing ID "test_user".'), + self.assertEqual(mock.call(enums.LogLevels.DEBUG, 'Assigned bucket 4242 to user with bucketing ID "test_user".'), mock_logging.call_args_list[2]) self.assertEqual( mock.call(enums.LogLevels.INFO, @@ -247,8 +241,7 @@ def test_bucket__experiment_in_group(self): 'test_user', 'test_user')) self.assertEqual(2, mock_logging.call_count) - self.assertEqual(mock.call(enums.LogLevels.DEBUG, 'Assigned bucket 8400 to user "test_user" ' - 'with bucketing ID "test_user".'), + self.assertEqual(mock.call(enums.LogLevels.DEBUG, 'Assigned bucket 8400 to user with bucketing ID "test_user".'), mock_logging.call_args_list[0]) self.assertEqual(mock.call(enums.LogLevels.INFO, 'User "test_user" is in no experiment.'), mock_logging.call_args_list[1]) @@ -260,13 +253,11 @@ def test_bucket__experiment_in_group(self): self.assertIsNone(self.bucketer.bucket( self.project_config.get_experiment_from_key('group_exp_1'), 'test_user', 'test_user')) self.assertEqual(4, mock_logging.call_count) - self.assertEqual(mock.call(enums.LogLevels.DEBUG, 'Assigned bucket 42 to user "test_user" ' - 'with bucketing ID "test_user".'), + self.assertEqual(mock.call(enums.LogLevels.DEBUG, 'Assigned bucket 42 to user with bucketing ID "test_user".'), mock_logging.call_args_list[0]) self.assertEqual(mock.call(enums.LogLevels.INFO, 'User "test_user" is in experiment group_exp_1 of group 19228.'), mock_logging.call_args_list[1]) - self.assertEqual(mock.call(enums.LogLevels.DEBUG, 'Assigned bucket 9500 to user "test_user" ' - 'with bucketing ID "test_user".'), + self.assertEqual(mock.call(enums.LogLevels.DEBUG, 'Assigned bucket 9500 to user with bucketing ID "test_user".'), mock_logging.call_args_list[2]) self.assertEqual(mock.call(enums.LogLevels.INFO, 'User "test_user" is in no variation.'), mock_logging.call_args_list[3]) @@ -279,8 +270,7 @@ def test_bucket__experiment_in_group(self): 'test_user', 'test_user')) self.assertEqual(2, mock_logging.call_count) - self.assertEqual(mock.call(enums.LogLevels.DEBUG, 'Assigned bucket 42 to user "test_user" ' - 'with bucketing ID "test_user".'), + self.assertEqual(mock.call(enums.LogLevels.DEBUG, 'Assigned bucket 42 to user with bucketing ID "test_user".'), mock_logging.call_args_list[0]) self.assertEqual( mock.call(enums.LogLevels.INFO, 'User "test_user" is not in experiment "group_exp_2" of group 19228.'), @@ -295,13 +285,11 @@ def test_bucket__experiment_in_group(self): 'test_user', 'test_user')) self.assertEqual(4, mock_logging.call_count) - self.assertEqual(mock.call(enums.LogLevels.DEBUG, 'Assigned bucket 42 to user "test_user" ' - 'with bucketing ID "test_user".'), + self.assertEqual(mock.call(enums.LogLevels.DEBUG, 'Assigned bucket 42 to user with bucketing ID "test_user".'), mock_logging.call_args_list[0]) self.assertEqual(mock.call(enums.LogLevels.INFO, 'User "test_user" is in experiment group_exp_1 of group 19228.'), mock_logging.call_args_list[1]) - self.assertEqual(mock.call(enums.LogLevels.DEBUG, 'Assigned bucket 424242 to user "test_user" ' - 'with bucketing ID "test_user".'), + self.assertEqual(mock.call(enums.LogLevels.DEBUG, 'Assigned bucket 424242 to user with bucketing ID "test_user".'), mock_logging.call_args_list[2]) self.assertEqual(mock.call(enums.LogLevels.INFO, 'User "test_user" is in no variation.'), mock_logging.call_args_list[3]) diff --git a/tests/test_decision_service.py b/tests/test_decision_service.py index 02bba912..3923bda8 100644 --- a/tests/test_decision_service.py +++ b/tests/test_decision_service.py @@ -101,7 +101,7 @@ def test_get_variation__experiment_not_running(self): mock.patch('optimizely.bucketer.Bucketer.bucket') as mock_bucket, \ mock.patch('optimizely.user_profile.UserProfileService.lookup') as mock_lookup, \ mock.patch('optimizely.user_profile.UserProfileService.save') as mock_save: - self.assertIsNone(self.decision_service.get_variation(experiment, 'user_1', None)) + self.assertIsNone(self.decision_service.get_variation(experiment, 'test_user', None)) mock_logging.assert_called_once_with(enums.LogLevels.INFO, 'Experiment "test_experiment" is not running.') # Assert no calls are made to other services @@ -140,10 +140,10 @@ def test_get_variation__user_forced_in_variation(self): mock.patch('optimizely.user_profile.UserProfileService.lookup') as mock_lookup, \ mock.patch('optimizely.user_profile.UserProfileService.save') as mock_save: self.assertEqual(entities.Variation('111128', 'control'), - self.decision_service.get_variation(experiment, 'user_1', None)) + self.decision_service.get_variation(experiment, 'test_user', None)) # Assert that forced variation is returned and stored decision or bucketing service are not involved - mock_get_forced_variation.assert_called_once_with(experiment, 'user_1') + mock_get_forced_variation.assert_called_once_with(experiment, 'test_user') self.assertEqual(0, mock_get_stored_variation.call_count) self.assertEqual(0, mock_audience_check.call_count) self.assertEqual(0, mock_bucket.call_count) @@ -606,9 +606,9 @@ def test_get_variation_for_feature__returns_none_for_user_in_group_experiment_no self.assertEqual(decision_service.Decision(expected_experiment, None, decision_service.DECISION_SOURCE_EXPERIMENT), - self.decision_service.get_variation_for_feature(feature, 'user_1')) + self.decision_service.get_variation_for_feature(feature, 'test_user')) - mock_decision.assert_called_once_with(self.project_config.get_group('19228'), 'user_1') + mock_decision.assert_called_once_with(self.project_config.get_group('19228'), 'test_user') def test_get_experiment_in_group(self, mock_logging): """ Test that get_experiment_in_group returns the bucketed experiment for the user. """ @@ -616,15 +616,17 @@ def test_get_experiment_in_group(self, mock_logging): group = self.project_config.get_group('19228') experiment = self.project_config.get_experiment_from_id('32222') with mock.patch('optimizely.bucketer.Bucketer.find_bucket', return_value='32222'): - self.assertEqual(experiment, self.decision_service.get_experiment_in_group(group, 'user_1')) + self.assertEqual(experiment, self.decision_service.get_experiment_in_group(group, 'test_user')) - mock_logging.assert_called_with(enums.LogLevels.INFO, 'User "user_1" is in experiment group_exp_1 of group 19228.') + mock_logging.assert_called_with(enums.LogLevels.INFO, 'User with bucketing ID "test_user" is in ' + 'experiment group_exp_1 of group 19228.') def test_get_experiment_in_group__returns_none_if_user_not_in_group(self, mock_logging): """ Test that get_experiment_in_group returns None if the user is not bucketed into the group. """ group = self.project_config.get_group('19228') with mock.patch('optimizely.bucketer.Bucketer.find_bucket', return_value=None): - self.assertIsNone(self.decision_service.get_experiment_in_group(group, 'user_1')) + self.assertIsNone(self.decision_service.get_experiment_in_group(group, 'test_user')) - mock_logging.assert_called_with(enums.LogLevels.INFO, 'User "user_1" is not in any experiments of group 19228.') + mock_logging.assert_called_with(enums.LogLevels.INFO, 'User with bucketing ID "test_user" is ' + 'not in any experiments of group 19228.')