Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions optimizely/decision_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -515,11 +515,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,
Expand All @@ -534,7 +529,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 user profile service
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:
Expand Down
2 changes: 1 addition & 1 deletion optimizely/event_dispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
9 changes: 1 addition & 8 deletions optimizely/odp/odp_event_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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}'))
Expand Down
63 changes: 8 additions & 55 deletions tests/test_decision_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -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_excludes_user_profile_service(self):
"""Test that CMAB decisions are not saved to user profile service."""

# Create a user context
user = optimizely_user_context.OptimizelyUserContext(
Expand All @@ -1096,7 +1096,7 @@ def test_get_variation_cmab_experiment_does_not_save_user_profile(self):
'Running',
'111150',
[], # No audience IDs
{},
{}, # No forced variations
[
entities.Variation('111151', 'variation_1'),
entities.Variation('111152', 'variation_2')
Expand All @@ -1115,8 +1115,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(user_profile_tracker, 'update_user_profile') as mock_update_profile:

# Configure CMAB service to return a decision
mock_cmab_service.get_decision.return_value = (
Expand All @@ -1136,62 +1135,16 @@ def test_get_variation_cmab_experiment_does_not_save_user_profile(self):
)
variation = variation_result['variation']
cmab_uuid = variation_result['cmab_uuid']
error = variation_result['error']

# Verify the variation and cmab_uuid are returned
# Verify the variation and cmab_uuid were returned correctly
self.assertEqual(entities.Variation('111151', 'variation_1'), variation)
self.assertEqual('test-cmab-uuid-123', cmab_uuid)
self.assertStrictFalse(error)

# Verify user profile was NOT updated for CMAB experiment
# Verify user profile was NOT updated (CMAB should be excluded)
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)

# Get a standard (non-CMAB) experiment
experiment = self.project_config.get_experiment_from_key("test_experiment")

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 user profile WAS updated for standard experiment
mock_update_profile.assert_called_once_with(experiment, variation)


class FeatureFlagDecisionTests(base.BaseTest):
def setUp(self):
Expand Down
12 changes: 4 additions & 8 deletions tests/test_odp_event_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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}).'
)
Expand All @@ -290,17 +288,15 @@ 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()
event_manager.event_queue.join()

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()
Expand Down
Loading