Skip to content

[AI-FSSDK] (DO NOT REVIEW) [FSSDK-12262] Exclude CMAB from UserProfileService #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jaeopt wants to merge 3 commits into from
Closed

[AI-FSSDK] (DO NOT REVIEW) [FSSDK-12262] Exclude CMAB from UserProfileService #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
39336ee
Revert "[FSSDK-12149] [Python] Add Event Retries (#475)"
jaeopt Feb 2, 2026
88b0644
Revert "[FSSDK-12035] Update: Exclude CMAB from UserProfileService (#…
jaeopt Feb 2, 2026
e6462ae
[FSSDK-12262] Exclude CMAB from UserProfileService
jaeopt Feb 2, 2026
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
12 changes: 5 additions & 7 deletions optimizely/decision_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 ' \
Expand Down Expand Up @@ -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,
Expand All @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion optimizely/event_dispatcher.py
View file
Open in desktop
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
View file
Open in desktop
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
81 changes: 21 additions & 60 deletions tests/test_decision_service.py
View file
Open in desktop
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_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(
Expand All @@ -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',
Expand All @@ -1108,15 +1104,20 @@ 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',
return_value=['$', []]), \
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 = (
Expand All @@ -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):
Expand Down
12 changes: 4 additions & 8 deletions tests/test_odp_event_manager.py
View file
Open in desktop
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