Skip to content

Commit 22c74ee

Browse files
[FSSDK-10763] Implement UPS request batching for decideForKeys (#440)
* update: UserProfile class created, changes in decision_service, decide_for_keys * update: get_variation function changed * update: new function in decision_service * update: everything implemented from java. tests are failing * update: minor changes * update: user_profile_tracker added to tests * update: some tests fixed * optimizely/decision_service.py -> Added check for `ignore_user_profile` in decision logic. optimizely/user_profile.py -> Improved user profile loading with missing key checks. tests/test_decision_service.py -> Updated tests to include user profile tracker. * tests/test_decision_service.py -> Added expected decision object. tests/test_decision_service.py -> Updated experiment bucket map call. tests/test_decision_service.py -> Introduced user_profile_tracker usage. tests/test_decision_service.py -> Modified method calls with user_profile_tracker. * optimizely/decision_service.py -> fixed get_variations_for_feature_list * optimizely/decision_service.py -> Fixed how rollout reasons are added tests/test_decision_service.py -> Added user profile tracker object * tests/test_user_context.py -> fixed some tests * optimizely/user_profile.py -> Updated type for `experiment_bucket_map`. tests/test_decision_service.py -> Fixed tests * all unit tests passing * lint check * fix: typechecks added * more types updated * all typechecks passing * gha typechecks fixed * all typecheck should pass * lint check should pass * removed unnecessary comments * removed comments from test * optimizely/decision_service.py -> Removed user profile save logic optimizely/optimizely.py -> Added loading and saving profile logic * optimizely/user_profile.py -> Updated experiment_bucket_map type optimizely/user_profile.py -> Testing user profile update logic * optimizely/decision_service.py -> Commented out profile loading optimizely/user_profile.py -> Removed unused import statement * optimizely/decision_service.py -> Removed unused profile loading optimizely/user_profile.py -> Fixed handling of reasons list optimizely/user_profile.py -> Improved profile retrieval error logging tests/test_decision_service.py -> Updated mock checks to simplify tests tests/test_user_profile.py -> Added tests for user profile handling tests/test_optimizely.py -> New test for variation lookup and save * optimizely/user_profile.py -> Reverted back to variation ID retrieval logic. * optimizely/user_profile.py -> Added error handling logic
1 parent 40880ff commit 22c74ee

7 files changed

+601
-477
lines changed

Diff for: optimizely/decision_service.py

+108-61
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
from .helpers import experiment as experiment_helper
2323
from .helpers import validator
2424
from .optimizely_user_context import OptimizelyUserContext, UserAttributes
25-
from .user_profile import UserProfile, UserProfileService
25+
from .user_profile import UserProfile, UserProfileService, UserProfileTracker
2626

2727
if TYPE_CHECKING:
2828
# prevent circular dependenacy by skipping import at runtime
@@ -35,7 +35,7 @@ class Decision(NamedTuple):
3535
None if no experiment/variation was selected."""
3636
experiment: Optional[entities.Experiment]
3737
variation: Optional[entities.Variation]
38-
source: str
38+
source: Optional[str]
3939

4040

4141
class DecisionService:
@@ -247,6 +247,8 @@ def get_variation(
247247
project_config: ProjectConfig,
248248
experiment: entities.Experiment,
249249
user_context: OptimizelyUserContext,
250+
user_profile_tracker: Optional[UserProfileTracker],
251+
reasons: list[str] = [],
250252
options: Optional[Sequence[str]] = None
251253
) -> tuple[Optional[entities.Variation], list[str]]:
252254
""" Top-level function to help determine variation user should be put in.
@@ -260,7 +262,9 @@ def get_variation(
260262
Args:
261263
project_config: Instance of ProjectConfig.
262264
experiment: Experiment for which user variation needs to be determined.
263-
user_context: contains user id and attributes
265+
user_context: contains user id and attributes.
266+
user_profile_tracker: tracker for reading and updating user profile of the user.
267+
reasons: Decision reasons.
264268
options: Decide options.
265269
266270
Returns:
@@ -275,6 +279,8 @@ def get_variation(
275279
ignore_user_profile = False
276280

277281
decide_reasons = []
282+
if reasons is not None:
283+
decide_reasons += reasons
278284
# Check if experiment is running
279285
if not experiment_helper.is_experiment_running(experiment):
280286
message = f'Experiment "{experiment.key}" is not running.'
@@ -296,23 +302,14 @@ def get_variation(
296302
return variation, decide_reasons
297303

298304
# Check to see if user has a decision available for the given experiment
299-
user_profile = UserProfile(user_id)
300-
if not ignore_user_profile and self.user_profile_service:
301-
try:
302-
retrieved_profile = self.user_profile_service.lookup(user_id)
303-
except:
304-
self.logger.exception(f'Unable to retrieve user profile for user "{user_id}" as lookup failed.')
305-
retrieved_profile = None
306-
307-
if retrieved_profile and validator.is_user_profile_valid(retrieved_profile):
308-
user_profile = UserProfile(**retrieved_profile)
309-
variation = self.get_stored_variation(project_config, experiment, user_profile)
310-
if variation:
311-
message = f'Returning previously activated variation ID "{variation}" of experiment ' \
312-
f'"{experiment}" for user "{user_id}" from user profile.'
313-
self.logger.info(message)
314-
decide_reasons.append(message)
315-
return variation, decide_reasons
305+
if user_profile_tracker is not None and not ignore_user_profile:
306+
variation = self.get_stored_variation(project_config, experiment, user_profile_tracker.get_user_profile())
307+
if variation:
308+
message = f'Returning previously activated variation ID "{variation}" of experiment ' \
309+
f'"{experiment}" for user "{user_id}" from user profile.'
310+
self.logger.info(message)
311+
decide_reasons.append(message)
312+
return variation, decide_reasons
316313
else:
317314
self.logger.warning('User profile has invalid format.')
318315

@@ -340,10 +337,9 @@ def get_variation(
340337
self.logger.info(message)
341338
decide_reasons.append(message)
342339
# Store this new decision and return the variation for the user
343-
if not ignore_user_profile and self.user_profile_service:
340+
if user_profile_tracker is not None and not ignore_user_profile:
344341
try:
345-
user_profile.save_variation_for_experiment(experiment.id, variation.id)
346-
self.user_profile_service.save(user_profile.__dict__)
342+
user_profile_tracker.update_user_profile(experiment, variation)
347343
except:
348344
self.logger.exception(f'Unable to save user profile for user "{user_id}".')
349345
return variation, decide_reasons
@@ -479,44 +475,7 @@ def get_variation_for_feature(
479475
Returns:
480476
Decision namedtuple consisting of experiment and variation for the user.
481477
"""
482-
decide_reasons = []
483-
484-
# Check if the feature flag is under an experiment and the the user is bucketed into one of these experiments
485-
if feature.experimentIds:
486-
# Evaluate each experiment ID and return the first bucketed experiment variation
487-
for experiment_id in feature.experimentIds:
488-
experiment = project_config.get_experiment_from_id(experiment_id)
489-
decision_variation = None
490-
491-
if experiment:
492-
optimizely_decision_context = OptimizelyUserContext.OptimizelyDecisionContext(feature.key,
493-
experiment.key)
494-
495-
forced_decision_variation, reasons_received = self.validated_forced_decision(
496-
project_config, optimizely_decision_context, user_context)
497-
decide_reasons += reasons_received
498-
499-
if forced_decision_variation:
500-
decision_variation = forced_decision_variation
501-
else:
502-
decision_variation, variation_reasons = self.get_variation(project_config,
503-
experiment, user_context, options)
504-
decide_reasons += variation_reasons
505-
506-
if decision_variation:
507-
message = f'User "{user_context.user_id}" bucketed into a ' \
508-
f'experiment "{experiment.key}" of feature "{feature.key}".'
509-
self.logger.debug(message)
510-
return Decision(experiment, decision_variation,
511-
enums.DecisionSources.FEATURE_TEST), decide_reasons
512-
513-
message = f'User "{user_context.user_id}" is not bucketed into any of the ' \
514-
f'experiments on the feature "{feature.key}".'
515-
self.logger.debug(message)
516-
variation, rollout_variation_reasons = self.get_variation_for_rollout(project_config, feature, user_context)
517-
if rollout_variation_reasons:
518-
decide_reasons += rollout_variation_reasons
519-
return variation, decide_reasons
478+
return self.get_variations_for_feature_list(project_config, [feature], user_context, options)[0]
520479

521480
def validated_forced_decision(
522481
self,
@@ -580,3 +539,91 @@ def validated_forced_decision(
580539
user_context.logger.info(user_has_forced_decision_but_invalid)
581540

582541
return None, reasons
542+
543+
def get_variations_for_feature_list(
544+
self,
545+
project_config: ProjectConfig,
546+
features: list[entities.FeatureFlag],
547+
user_context: OptimizelyUserContext,
548+
options: Optional[Sequence[str]] = None
549+
) -> list[tuple[Decision, list[str]]]:
550+
"""
551+
Returns the list of experiment/variation the user is bucketed in for the given list of features.
552+
Args:
553+
project_config: Instance of ProjectConfig.
554+
features: List of features for which we are determining if it is enabled or not for the given user.
555+
user_context: user context for user.
556+
options: Decide options.
557+
558+
Returns:
559+
List of Decision namedtuple consisting of experiment and variation for the user.
560+
"""
561+
decide_reasons: list[str] = []
562+
563+
if options:
564+
ignore_ups = OptimizelyDecideOption.IGNORE_USER_PROFILE_SERVICE in options
565+
else:
566+
ignore_ups = False
567+
568+
user_profile_tracker: Optional[UserProfileTracker] = None
569+
if self.user_profile_service is not None and not ignore_ups:
570+
user_profile_tracker = UserProfileTracker(user_context.user_id, self.user_profile_service, self.logger)
571+
user_profile_tracker.load_user_profile(decide_reasons, None)
572+
573+
decisions = []
574+
575+
for feature in features:
576+
feature_reasons = decide_reasons.copy()
577+
experiment_decision_found = False # Track if an experiment decision was made for the feature
578+
579+
# Check if the feature flag is under an experiment
580+
if feature.experimentIds:
581+
for experiment_id in feature.experimentIds:
582+
experiment = project_config.get_experiment_from_id(experiment_id)
583+
decision_variation = None
584+
585+
if experiment:
586+
optimizely_decision_context = OptimizelyUserContext.OptimizelyDecisionContext(
587+
feature.key, experiment.key)
588+
forced_decision_variation, reasons_received = self.validated_forced_decision(
589+
project_config, optimizely_decision_context, user_context)
590+
feature_reasons.extend(reasons_received)
591+
592+
if forced_decision_variation:
593+
decision_variation = forced_decision_variation
594+
else:
595+
decision_variation, variation_reasons = self.get_variation(
596+
project_config, experiment, user_context, user_profile_tracker, feature_reasons, options
597+
)
598+
feature_reasons.extend(variation_reasons)
599+
600+
if decision_variation:
601+
self.logger.debug(
602+
f'User "{user_context.user_id}" '
603+
f'bucketed into experiment "{experiment.key}" of feature "{feature.key}".'
604+
)
605+
decision = Decision(experiment, decision_variation, enums.DecisionSources.FEATURE_TEST)
606+
decisions.append((decision, feature_reasons))
607+
experiment_decision_found = True # Mark that a decision was found
608+
break # Stop after the first successful experiment decision
609+
610+
# Only process rollout if no experiment decision was found
611+
if not experiment_decision_found:
612+
rollout_decision, rollout_reasons = self.get_variation_for_rollout(project_config,
613+
feature,
614+
user_context)
615+
if rollout_reasons:
616+
feature_reasons.extend(rollout_reasons)
617+
if rollout_decision:
618+
self.logger.debug(f'User "{user_context.user_id}" '
619+
f'bucketed into rollout for feature "{feature.key}".')
620+
else:
621+
self.logger.debug(f'User "{user_context.user_id}" '
622+
f'not bucketed into any rollout for feature "{feature.key}".')
623+
624+
decisions.append((rollout_decision, feature_reasons))
625+
626+
if self.user_profile_service is not None and user_profile_tracker is not None and ignore_ups is False:
627+
user_profile_tracker.save_user_profile()
628+
629+
return decisions

0 commit comments

Comments
 (0)