Skip to content

Commit e6ad886

Browse files
committed
Implement Ruby changes correctly
1 parent 232d26c commit e6ad886

File tree

3 files changed

+39
-70
lines changed

3 files changed

+39
-70
lines changed

optimizely/decision_service.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -708,14 +708,14 @@ def get_decision_for_flag(
708708
user_id = user_context.user_id
709709

710710
# Check holdouts
711-
holdouts = project_config.get_holdouts_for_flag(feature_flag.id)
711+
holdouts = project_config.get_holdouts_for_flag(feature_flag.key)
712712
for holdout in holdouts:
713713
holdout_decision = self.get_variation_for_holdout(holdout, user_context, project_config)
714714
reasons.extend(holdout_decision['reasons'])
715715

716716
decision = holdout_decision['decision']
717717
# Check if user was bucketed into holdout (has a variation)
718-
if decision is None or decision.variation is None:
718+
if decision.variation is None:
719719
continue
720720

721721
message = (
@@ -905,9 +905,9 @@ def get_variation_for_holdout(
905905
self.logger.info(message)
906906
decide_reasons.append(message)
907907

908-
# Create Decision for holdout - pass holdout dict as experiment, source is HOLDOUT
908+
# Create Decision for holdout - experiment is None, source is HOLDOUT
909909
holdout_decision: Decision = Decision(
910-
experiment=holdout, # type: ignore[arg-type]
910+
experiment=None,
911911
variation=variation,
912912
source=enums.DecisionSources.HOLDOUT,
913913
cmab_uuid=None
@@ -1017,7 +1017,7 @@ def get_variations_for_feature_list(
10171017
ignore_ups = OptimizelyDecideOption.IGNORE_USER_PROFILE_SERVICE in options
10181018

10191019
user_profile_tracker: Optional[UserProfileTracker] = None
1020-
if not ignore_ups and self.user_profile_service:
1020+
if self.user_profile_service is not None and not ignore_ups:
10211021
user_id = user_context.user_id
10221022
user_profile_tracker = UserProfileTracker(user_id, self.user_profile_service, self.logger)
10231023
user_profile_tracker.load_user_profile([], None)

optimizely/optimizely.py

Lines changed: 18 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515

1616
from typing import TYPE_CHECKING, Any, Optional, Union
1717

18-
from optimizely.helpers.types import HoldoutDict, VariationDict
18+
from optimizely.helpers.types import VariationDict
1919

2020

2121
from . import decision_service
@@ -261,25 +261,6 @@ def _get_feature_enabled(self, variation: Optional[Union[entities.Variation, Var
261261
except (AttributeError, KeyError, TypeError):
262262
return False
263263

264-
def _get_experiment_key(self, experiment: Optional[Union[entities.Experiment, HoldoutDict]]) -> Optional[str]:
265-
"""Helper to extract experiment/holdout key from either dict or Experiment object.
266-
Args:
267-
experiment: Either a HoldoutDict (from holdout) or entities.Experiment object
268-
Returns:
269-
The experiment/holdout key as a string, or None if not available
270-
"""
271-
if experiment is None:
272-
return None
273-
274-
try:
275-
if isinstance(experiment, dict):
276-
return experiment.get('key')
277-
else:
278-
return experiment.key
279-
except (AttributeError, KeyError, TypeError):
280-
self.logger.warning(f"Unable to extract experiment key from {type(experiment)}")
281-
return None
282-
283264
def _validate_instantiation_options(self) -> None:
284265
""" Helper method to validate all instantiation parameters.
285266
@@ -452,7 +433,7 @@ def _get_feature_variable_for_type(
452433
decision_result = self.decision_service.get_variation_for_feature(project_config, feature_flag, user_context)
453434
decision = decision_result['decision']
454435

455-
if decision and decision.variation:
436+
if decision.variation:
456437

457438
feature_enabled = self._get_feature_enabled(decision.variation)
458439
if feature_enabled:
@@ -474,7 +455,7 @@ def _get_feature_variable_for_type(
474455

475456
if decision.source in (enums.DecisionSources.FEATURE_TEST, enums.DecisionSources.HOLDOUT):
476457
source_info = {
477-
'experiment_key': self._get_experiment_key(decision.experiment),
458+
'experiment_key': decision.experiment.key if decision.experiment else None,
478459
'variation_key': self._get_variation_key(decision.variation),
479460
}
480461

@@ -578,7 +559,7 @@ def _get_all_feature_variables_for_type(
578559

579560
if decision.source == enums.DecisionSources.FEATURE_TEST:
580561
source_info = {
581-
'experiment_key': self._get_experiment_key(decision.experiment),
562+
'experiment_key': decision.experiment.key if decision.experiment else None,
582563
'variation_key': self._get_variation_key(decision.variation),
583564
}
584565

@@ -811,33 +792,30 @@ def is_feature_enabled(self, feature_key: str, user_id: str, attributes: Optiona
811792
user_context = OptimizelyUserContext(self, self.logger, user_id, attributes, False)
812793

813794
decision = self.decision_service.get_variation_for_feature(project_config, feature, user_context)['decision']
814-
cmab_uuid = decision.cmab_uuid if decision else None
795+
cmab_uuid = decision.cmab_uuid
815796

816-
is_source_experiment = decision.source == enums.DecisionSources.FEATURE_TEST if decision else False
817-
is_source_rollout = decision.source == enums.DecisionSources.ROLLOUT if decision else False
797+
is_source_experiment = decision.source == enums.DecisionSources.FEATURE_TEST
798+
is_source_rollout = decision.source == enums.DecisionSources.ROLLOUT
818799

819-
if decision and decision.variation:
800+
if decision.variation:
820801
if self._get_feature_enabled(decision.variation) is True:
821802
feature_enabled = True
822803

823-
if (decision and (is_source_rollout or not decision.variation) and
824-
project_config.get_send_flag_decisions_value()):
804+
if (is_source_rollout or not decision.variation) and project_config.get_send_flag_decisions_value():
825805
self._send_impression_event(
826-
project_config, decision.experiment, decision.variation, feature.key,
827-
self._get_experiment_key(decision.experiment) or '', str(decision.source), feature_enabled,
828-
user_id, attributes, cmab_uuid
806+
project_config, decision.experiment, decision.variation, feature.key, decision.experiment.key if
807+
decision.experiment else '', str(decision.source), feature_enabled, user_id, attributes, cmab_uuid
829808
)
830809

831810
# Send event if Decision came from an experiment.
832-
if decision and is_source_experiment and decision.variation and decision.experiment:
811+
if is_source_experiment and decision.variation and decision.experiment:
833812
source_info = {
834-
'experiment_key': self._get_experiment_key(decision.experiment),
813+
'experiment_key': decision.experiment.key,
835814
'variation_key': self._get_variation_key(decision.variation),
836815
}
837816
self._send_impression_event(
838-
project_config, decision.experiment, decision.variation, feature.key,
839-
self._get_experiment_key(decision.experiment) or '', str(decision.source), feature_enabled,
840-
user_id, attributes, cmab_uuid
817+
project_config, decision.experiment, decision.variation, feature.key, decision.experiment.key,
818+
str(decision.source), feature_enabled, user_id, attributes, cmab_uuid
841819
)
842820

843821
if feature_enabled:
@@ -1266,17 +1244,17 @@ def _create_optimizely_decision(
12661244
) -> OptimizelyDecision:
12671245
user_id = user_context.user_id
12681246
feature_enabled = False
1269-
if flag_decision and flag_decision.variation is not None:
1247+
if flag_decision.variation is not None:
12701248
if self._get_feature_enabled(flag_decision.variation):
12711249
feature_enabled = True
12721250

12731251
self.logger.info(f'Feature {flag_key} is enabled for user {user_id} {feature_enabled}"')
12741252

12751253
# Create Optimizely Decision Result.
12761254
attributes = user_context.get_user_attributes()
1277-
rule_key = self._get_experiment_key(flag_decision.experiment) if flag_decision else None
1255+
rule_key = flag_decision.experiment.key if flag_decision.experiment else None
12781256
all_variables = {}
1279-
decision_source = flag_decision.source if flag_decision else None
1257+
decision_source = flag_decision.source
12801258
decision_event_dispatched = False
12811259

12821260
feature_flag = project_config.feature_key_map.get(flag_key)

optimizely/project_config.py

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -833,55 +833,46 @@ def get_flag_variation(
833833

834834
return None
835835

836-
def get_holdouts_for_flag(self, flag_key_or_id: str) -> list[HoldoutDict]:
836+
def get_holdouts_for_flag(self, flag_key: str) -> list[HoldoutDict]:
837837
""" Helper method to get holdouts from an applied feature flag.
838838
839839
Args:
840-
flag_key_or_id: Key or ID of the feature flag.
841-
This parameter is required and should not be null/None.
840+
flag_key: Key of the feature flag.
841+
This parameter is required and should not be null/None.
842842
843843
Returns:
844844
The holdouts that apply for a specific flag.
845845
"""
846846
if not self.holdouts:
847847
return []
848848

849-
# Check cache first (before validation, so we cache the validation result too)
850-
if flag_key_or_id in self.flag_holdouts_map:
851-
return self.flag_holdouts_map[flag_key_or_id]
852-
853-
# Find the flag by key or ID
854-
flag_id = None
855-
for flag in self.feature_flags:
856-
if flag['id'] == flag_key_or_id or flag['key'] == flag_key_or_id:
857-
flag_id = flag['id']
858-
break
859-
860-
if flag_id is None:
861-
# Cache the empty result for non-existent flags
862-
self.flag_holdouts_map[flag_key_or_id] = []
849+
# Get the feature flag to extract flag_id
850+
feature = self.feature_key_map.get(flag_key)
851+
if not feature:
863852
return []
864853

854+
flag_id = feature.id
855+
856+
# Check cache first (before validation, so we cache the validation result too)
857+
if flag_id in self.flag_holdouts_map:
858+
return self.flag_holdouts_map[flag_id]
859+
865860
# Prioritize global holdouts first
866861
excluded = self.excluded_holdouts.get(flag_id, [])
867862

868863
if excluded:
869-
# Filter out excluded holdouts from global holdouts
870-
active_holdouts = [
871-
holdout for holdout in self.global_holdouts
872-
if holdout not in excluded
873-
]
864+
active_holdouts = [holdout for holdout in self.global_holdouts if holdout not in excluded]
874865
else:
875866
active_holdouts = self.global_holdouts.copy()
876867

877868
# Append included holdouts
878869
included = self.included_holdouts.get(flag_id, [])
879870
active_holdouts.extend(included)
880871

881-
# Cache the result using the input parameter as the cache key
882-
self.flag_holdouts_map[flag_key_or_id] = active_holdouts
872+
# Cache the result
873+
self.flag_holdouts_map[flag_id] = active_holdouts
883874

884-
return self.flag_holdouts_map[flag_key_or_id]
875+
return self.flag_holdouts_map[flag_id]
885876

886877
def get_holdout(self, holdout_id: str) -> Optional[HoldoutDict]:
887878
""" Helper method to get holdout from holdout ID.

0 commit comments

Comments
 (0)