diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index be2be2c..6d7e73c 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -457,7 +457,9 @@ def get_variation( } # Check to see if user has a decision available for the given experiment - if user_profile_tracker is not None and not ignore_user_profile: + # Exclude CMAB experiments from UPS logic as CMAB requires dynamic decisions + # that consider TTL and user attributes + if user_profile_tracker is not None and not ignore_user_profile and not experiment.cmab: variation = self.get_stored_variation(project_config, experiment, user_profile_tracker.get_user_profile()) if variation: message = f'Returning previously activated variation ID "{variation}" of experiment ' \ @@ -515,11 +517,6 @@ def get_variation( 'reasons': decide_reasons, 'variation': None } - ignore_user_profile = True - self.logger.debug( - f'Skipping user profile service for CMAB experiment "{experiment.key}". ' - f'CMAB decisions are dynamic and not stored for sticky bucketing.' - ) variation_id = cmab_decision['variation_id'] if cmab_decision else None cmab_uuid = cmab_decision['cmab_uuid'] if cmab_decision else None variation = project_config.get_variation_from_id(experiment_key=experiment.key, @@ -534,7 +531,8 @@ def get_variation( self.logger.info(message) decide_reasons.append(message) # Store this new decision and return the variation for the user - if user_profile_tracker is not None and not ignore_user_profile: + # Exclude CMAB experiments from UPS logic as CMAB requires dynamic decisions + if user_profile_tracker is not None and not ignore_user_profile and not experiment.cmab: try: user_profile_tracker.update_user_profile(experiment, variation) except: diff --git a/optimizely/event_dispatcher.py b/optimizely/event_dispatcher.py index b06b9e1..55209dc 100644 --- a/optimizely/event_dispatcher.py +++ b/optimizely/event_dispatcher.py @@ -49,7 +49,7 @@ def dispatch_event(event: event_builder.Event) -> None: session = requests.Session() retries = Retry(total=EventDispatchConfig.RETRIES, - backoff_factor=0.2, + backoff_factor=0.1, status_forcelist=[500, 502, 503, 504]) adapter = HTTPAdapter(max_retries=retries) diff --git a/optimizely/odp/odp_event_manager.py b/optimizely/odp/odp_event_manager.py index 3fb961a..85512e9 100644 --- a/optimizely/odp/odp_event_manager.py +++ b/optimizely/odp/odp_event_manager.py @@ -163,8 +163,6 @@ def _flush_batch(self) -> None: self.logger.debug(f'ODP event queue: flushing batch size {batch_len}.') should_retry = False - initial_retry_interval = 0.2 # 200ms - max_retry_interval = 1.0 # 1 second for i in range(1 + self.retry_count): try: @@ -178,12 +176,7 @@ def _flush_batch(self) -> None: if not should_retry: break if i < self.retry_count: - # Exponential backoff: 200ms, 400ms, 800ms, ... capped at 1s - delay = initial_retry_interval * (2 ** i) - if delay > max_retry_interval: - delay = max_retry_interval - self.logger.debug(f'Error dispatching ODP events, retrying after {delay}s.') - time.sleep(delay) + self.logger.debug('Error dispatching ODP events, scheduled to retry.') if should_retry: self.logger.error(Errors.ODP_EVENT_FAILED.format(f'Failed after {i} retries: {self._current_batch}')) diff --git a/tests/test_decision_service.py b/tests/test_decision_service.py index b38a03b..a921434 100644 --- a/tests/test_decision_service.py +++ b/tests/test_decision_service.py @@ -1074,8 +1074,8 @@ def test_get_variation_cmab_experiment_with_whitelisted_variation(self): mock_bucket.assert_not_called() mock_cmab_decision.assert_not_called() - def test_get_variation_cmab_experiment_does_not_save_user_profile(self): - """Test that CMAB experiments do not save bucketing decisions to user profile.""" + def test_get_variation_cmab_experiment_ignores_user_profile_service(self): + """Test get_variation with CMAB experiment does not use UPS for sticky bucketing.""" # Create a user context user = optimizely_user_context.OptimizelyUserContext( @@ -1085,10 +1085,6 @@ def test_get_variation_cmab_experiment_does_not_save_user_profile(self): user_attributes={} ) - # Create a user profile service and tracker - user_profile_service = user_profile.UserProfileService() - user_profile_tracker = user_profile.UserProfileTracker(user.user_id, user_profile_service) - # Create a CMAB experiment cmab_experiment = entities.Experiment( '111150', @@ -1108,6 +1104,12 @@ def test_get_variation_cmab_experiment_does_not_save_user_profile(self): cmab={'trafficAllocation': 5000} ) + # Create a mock user profile tracker with a stored decision + mock_user_profile_tracker = mock.MagicMock() + mock_user_profile = mock.MagicMock() + mock_user_profile.get_variation_for_experiment.return_value = '111152' + mock_user_profile_tracker.get_user_profile.return_value = mock_user_profile + with mock.patch('optimizely.helpers.experiment.is_experiment_running', return_value=True), \ mock.patch('optimizely.helpers.audience.does_user_meet_audience_conditions', return_value=[True, []]), \ mock.patch.object(self.decision_service.bucketer, 'bucket_to_entity_id', @@ -1115,8 +1117,7 @@ def test_get_variation_cmab_experiment_does_not_save_user_profile(self): mock.patch.object(self.decision_service, 'cmab_service') as mock_cmab_service, \ mock.patch.object(self.project_config, 'get_variation_from_id', return_value=entities.Variation('111151', 'variation_1')), \ - mock.patch.object(user_profile_tracker, 'update_user_profile') as mock_update_profile, \ - mock.patch.object(self.decision_service, 'logger') as mock_logger: + mock.patch.object(self.decision_service, 'get_stored_variation') as mock_get_stored: # Configure CMAB service to return a decision mock_cmab_service.get_decision.return_value = ( @@ -1132,65 +1133,25 @@ def test_get_variation_cmab_experiment_does_not_save_user_profile(self): self.project_config, cmab_experiment, user, - user_profile_tracker + mock_user_profile_tracker ) variation = variation_result['variation'] cmab_uuid = variation_result['cmab_uuid'] + error = variation_result['error'] - # Verify the variation and cmab_uuid are returned - self.assertEqual(entities.Variation('111151', 'variation_1'), variation) - self.assertEqual('test-cmab-uuid-123', cmab_uuid) - - # Verify user profile was NOT updated for CMAB experiment - mock_update_profile.assert_not_called() - - # Verify debug log was called to explain CMAB exclusion - mock_logger.debug.assert_any_call( - 'Skipping user profile service for CMAB experiment "cmab_experiment". ' - 'CMAB decisions are dynamic and not stored for sticky bucketing.' - ) - - def test_get_variation_standard_experiment_saves_user_profile(self): - """Test that standard (non-CMAB) experiments DO save bucketing decisions to user profile.""" - - user = optimizely_user_context.OptimizelyUserContext( - optimizely_client=None, - logger=None, - user_id="test_user", - user_attributes={} - ) - - # Create a user profile service and tracker - user_profile_service = user_profile.UserProfileService() - user_profile_tracker = user_profile.UserProfileTracker(user.user_id, user_profile_service) + # Verify that get_stored_variation was NOT called (CMAB should skip UPS) + mock_get_stored.assert_not_called() - # Get a standard (non-CMAB) experiment - experiment = self.project_config.get_experiment_from_key("test_experiment") + # Verify that CMAB service was called (CMAB decision was made) + mock_cmab_service.get_decision.assert_called_once() - with mock.patch('optimizely.decision_service.DecisionService.get_whitelisted_variation', - return_value=[None, []]), \ - mock.patch('optimizely.decision_service.DecisionService.get_stored_variation', - return_value=None), \ - mock.patch('optimizely.helpers.audience.does_user_meet_audience_conditions', - return_value=[True, []]), \ - mock.patch('optimizely.bucketer.Bucketer.bucket', - return_value=[entities.Variation("111129", "variation"), []]), \ - mock.patch.object(user_profile_tracker, 'update_user_profile') as mock_update_profile: - - # Call get_variation with standard experiment and user profile tracker - variation_result = self.decision_service.get_variation( - self.project_config, - experiment, - user, - user_profile_tracker - ) - variation = variation_result['variation'] - - # Verify variation was returned - self.assertEqual(entities.Variation("111129", "variation"), variation) + # Verify variation came from CMAB service, not UPS + self.assertEqual(entities.Variation('111151', 'variation_1'), variation) + self.assertEqual('test-cmab-uuid-123', cmab_uuid) + self.assertStrictFalse(error) - # Verify user profile WAS updated for standard experiment - mock_update_profile.assert_called_once_with(experiment, variation) + # Verify that update_user_profile was NOT called (CMAB should not save to UPS) + mock_user_profile_tracker.update_user_profile.assert_not_called() class FeatureFlagDecisionTests(base.BaseTest): diff --git a/tests/test_odp_event_manager.py b/tests/test_odp_event_manager.py index acec396..d9d29ea 100644 --- a/tests/test_odp_event_manager.py +++ b/tests/test_odp_event_manager.py @@ -265,7 +265,7 @@ def test_odp_event_manager_retry_failure(self, *args): with mock.patch.object( event_manager.api_manager, 'send_odp_events', new_callable=CopyingMock, return_value=True - ) as mock_send, mock.patch('time.sleep') as mock_sleep: + ) as mock_send: event_manager.send_event(**self.events[0]) event_manager.send_event(**self.events[1]) event_manager.flush() @@ -275,9 +275,7 @@ def test_odp_event_manager_retry_failure(self, *args): [mock.call(self.api_key, self.api_host, self.processed_events)] * number_of_tries ) self.assertEqual(len(event_manager._current_batch), 0) - # Verify exponential backoff delays: 0.2s, 0.4s, 0.8s - mock_sleep.assert_has_calls([mock.call(0.2), mock.call(0.4), mock.call(0.8)]) - mock_logger.debug.assert_any_call('Error dispatching ODP events, retrying after 0.2s.') + mock_logger.debug.assert_any_call('Error dispatching ODP events, scheduled to retry.') mock_logger.error.assert_called_once_with( f'ODP event send failed (Failed after 3 retries: {self.processed_events}).' ) @@ -290,7 +288,7 @@ def test_odp_event_manager_retry_success(self, *args): with mock.patch.object( event_manager.api_manager, 'send_odp_events', new_callable=CopyingMock, side_effect=[True, True, False] - ) as mock_send, mock.patch('time.sleep') as mock_sleep: + ) as mock_send: event_manager.send_event(**self.events[0]) event_manager.send_event(**self.events[1]) event_manager.flush() @@ -298,9 +296,7 @@ def test_odp_event_manager_retry_success(self, *args): mock_send.assert_has_calls([mock.call(self.api_key, self.api_host, self.processed_events)] * 3) self.assertEqual(len(event_manager._current_batch), 0) - # Verify exponential backoff delays: 0.2s, 0.4s (only 2 delays for 3 attempts) - mock_sleep.assert_has_calls([mock.call(0.2), mock.call(0.4)]) - mock_logger.debug.assert_any_call('Error dispatching ODP events, retrying after 0.2s.') + mock_logger.debug.assert_any_call('Error dispatching ODP events, scheduled to retry.') mock_logger.error.assert_not_called() self.assertStrictTrue(event_manager.is_running) event_manager.stop()