Skip to content

Commit c2b3d96

Browse files
-Error propagated to optimizely.py
-test cases changed to handle return type dicts of DecisionResult and VariationResult
1 parent 6ca1102 commit c2b3d96

File tree

5 files changed

+416
-328
lines changed

5 files changed

+416
-328
lines changed

optimizely/decision_service.py

Lines changed: 47 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,6 @@ def get_variation(
363363
b. If so, fetch the CMAB decision and assign the corresponding variation and cmab_uuid.
364364
7. For non-CMAB experiments, bucket the user into a variation.
365365
8. If a variation is assigned, optionally update the user profile.
366-
9. Return the assigned variation, decision reasons, and cmab_uuid (if applicable).
367366
368367
Args:
369368
project_config: Instance of ProjectConfig.
@@ -374,10 +373,11 @@ def get_variation(
374373
options: Decide options.
375374
376375
Returns:
377-
A tuple of:
378-
- The assigned Variation (or None if not assigned).
379-
- A list of log messages representing decision making.
380-
- The cmab_uuid if the experiment is a CMAB experiment, otherwise None.
376+
A VariationResult dictionary with:
377+
- 'variation': The assigned Variation (or None if not assigned).
378+
- 'reasons': A list of log messages representing decision making.
379+
- 'cmab_uuid': The cmab_uuid if the experiment is a CMAB experiment, otherwise None.
380+
- 'error': Boolean indicating if an error occurred during the decision process.
381381
"""
382382
user_id = user_context.user_id
383383
if options:
@@ -657,7 +657,7 @@ def get_variation_for_feature(
657657
feature: entities.FeatureFlag,
658658
user_context: OptimizelyUserContext,
659659
options: Optional[list[str]] = None
660-
) -> tuple[Decision, list[str]]:
660+
) -> DecisionResult:
661661
""" Returns the experiment/variation the user is bucketed in for the given feature.
662662
663663
Args:
@@ -667,8 +667,11 @@ def get_variation_for_feature(
667667
options: Decide options.
668668
669669
Returns:
670-
Decision namedtuple consisting of experiment and variation for the user.
671-
"""
670+
A DecisionResult dictionary containing:
671+
- 'decision': Decision namedtuple with experiment, variation, source, and cmab_uuid.
672+
- 'error': Boolean indicating if an error occurred during the decision process.
673+
- 'reasons': List of log messages representing decision making for the feature.
674+
"""
672675
return self.get_variations_for_feature_list(project_config, [feature], user_context, options)[0]
673676

674677
def validated_forced_decision(
@@ -740,17 +743,21 @@ def get_variations_for_feature_list(
740743
features: list[entities.FeatureFlag],
741744
user_context: OptimizelyUserContext,
742745
options: Optional[Sequence[str]] = None
743-
) -> list[tuple[Decision, list[str]]]:
746+
) -> list[DecisionResult]:
744747
"""
745748
Returns the list of experiment/variation the user is bucketed in for the given list of features.
749+
746750
Args:
747-
project_config: Instance of ProjectConfig.
748-
features: List of features for which we are determining if it is enabled or not for the given user.
749-
user_context: user context for user.
750-
options: Decide options.
751+
project_config: Instance of ProjectConfig.
752+
features: List of features for which we are determining if it is enabled or not for the given user.
753+
user_context: user context for user.
754+
options: Decide options.
751755
752756
Returns:
753-
List of Decision namedtuple consisting of experiment and variation for the user.
757+
A list of DecisionResult dictionaries, each containing:
758+
- 'decision': Decision namedtuple with experiment, variation, source, and cmab_uuid.
759+
- 'error': Boolean indicating if an error occurred during the decision process.
760+
- 'reasons': List of log messages representing decision making for each feature.
754761
"""
755762
decide_reasons: list[str] = []
756763

@@ -786,27 +793,45 @@ def get_variations_for_feature_list(
786793
if forced_decision_variation:
787794
decision_variation = forced_decision_variation
788795
cmab_uuid = None
796+
error = False
789797
else:
790798
variation_result = self.get_variation(
791799
project_config, experiment, user_context, user_profile_tracker, feature_reasons, options
792800
)
793801
cmab_uuid = variation_result['cmab_uuid']
794802
variation_reasons = variation_result['reasons']
795803
decision_variation = variation_result['variation']
804+
error = variation_result['error']
796805
feature_reasons.extend(variation_reasons)
797806

807+
if error:
808+
decision = Decision(experiment, None, enums.DecisionSources.FEATURE_TEST, cmab_uuid)
809+
decision_result: DecisionResult = {
810+
'decision': decision,
811+
'error': True,
812+
'reasons': feature_reasons
813+
}
814+
decisions.append(decision_result)
815+
experiment_decision_found = True
816+
break
817+
798818
if decision_variation:
799819
self.logger.debug(
800820
f'User "{user_context.user_id}" '
801821
f'bucketed into experiment "{experiment.key}" of feature "{feature.key}".'
802822
)
803823
decision = Decision(experiment, decision_variation,
804824
enums.DecisionSources.FEATURE_TEST, cmab_uuid)
805-
decisions.append((decision, feature_reasons))
825+
decision_result = {
826+
'decision': decision,
827+
'error': False,
828+
'reasons': feature_reasons
829+
}
830+
decisions.append(decision_result)
806831
experiment_decision_found = True # Mark that a decision was found
807832
break # Stop after the first successful experiment decision
808833

809-
# Only process rollout if no experiment decision was found
834+
# Only process rollout if no experiment decision was found and no error
810835
if not experiment_decision_found:
811836
rollout_decision, rollout_reasons = self.get_variation_for_rollout(project_config,
812837
feature,
@@ -820,7 +845,12 @@ def get_variations_for_feature_list(
820845
self.logger.debug(f'User "{user_context.user_id}" '
821846
f'not bucketed into any rollout for feature "{feature.key}".')
822847

823-
decisions.append((rollout_decision, feature_reasons))
848+
decision_result = {
849+
'decision': rollout_decision,
850+
'error': False,
851+
'reasons': feature_reasons
852+
}
853+
decisions.append(decision_result)
824854

825855
if self.user_profile_service is not None and user_profile_tracker is not None and ignore_ups is False:
826856
user_profile_tracker.save_user_profile()

optimizely/optimizely.py

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,8 @@ def _get_feature_variable_for_type(
357357

358358
user_context = OptimizelyUserContext(self, self.logger, user_id, attributes, False)
359359

360-
decision, _ = self.decision_service.get_variation_for_feature(project_config, feature_flag, user_context)
360+
decision_result = self.decision_service.get_variation_for_feature(project_config, feature_flag, user_context)
361+
decision = decision_result['decision']
361362

362363
if decision.variation:
363364

@@ -444,7 +445,9 @@ def _get_all_feature_variables_for_type(
444445

445446
user_context = OptimizelyUserContext(self, self.logger, user_id, attributes, False)
446447

447-
decision, _ = self.decision_service.get_variation_for_feature(project_config, feature_flag, user_context)
448+
decision = self.decision_service.get_variation_for_feature(project_config,
449+
feature_flag,
450+
user_context)['decision']
448451

449452
if decision.variation:
450453

@@ -715,7 +718,7 @@ def is_feature_enabled(self, feature_key: str, user_id: str, attributes: Optiona
715718

716719
user_context = OptimizelyUserContext(self, self.logger, user_id, attributes, False)
717720

718-
decision, _ = self.decision_service.get_variation_for_feature(project_config, feature, user_context)
721+
decision = self.decision_service.get_variation_for_feature(project_config, feature, user_context)['decision']
719722
is_source_experiment = decision.source == enums.DecisionSources.FEATURE_TEST
720723
is_source_rollout = decision.source == enums.DecisionSources.ROLLOUT
721724

@@ -1368,8 +1371,11 @@ def _decide_for_keys(
13681371
)
13691372

13701373
for i in range(0, len(flags_without_forced_decision)):
1371-
decision = decision_list[i][0]
1372-
reasons = decision_list[i][1]
1374+
decision = decision_list[i]['decision']
1375+
reasons = decision_list[i]['reasons']
1376+
# Can catch errors now. Not used as decision logic implicitly handles error decision.
1377+
# Will be required for impression events
1378+
# error = decision_list[i]['error']
13731379
flag_key = flags_without_forced_decision[i].key
13741380
flag_decisions[flag_key] = decision
13751381
decision_reasons_dict[flag_key] += reasons

0 commit comments

Comments
 (0)