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
25 changes: 1 addition & 24 deletions optimizely/decision_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -733,30 +733,7 @@ def get_decision_for_flag(
reasons = decide_reasons.copy() if decide_reasons else []
user_id = user_context.user_id

# Check holdouts
holdouts = project_config.get_holdouts_for_flag(feature_flag.key)
for holdout in holdouts:
holdout_decision = self.get_variation_for_holdout(holdout, user_context, project_config)
reasons.extend(holdout_decision['reasons'])

decision = holdout_decision['decision']
# Check if user was bucketed into holdout (has a variation)
if decision.variation is None:
continue

message = (
f"The user '{user_id}' is bucketed into holdout '{holdout.key}' "
f"for feature flag '{feature_flag.key}'."
)
self.logger.info(message)
reasons.append(message)
return {
'decision': holdout_decision['decision'],
'error': False,
'reasons': reasons
}

# If no holdout decision, check experiments then rollouts
# Check experiments then rollouts
if feature_flag.experimentIds:
for experiment_id in feature_flag.experimentIds:
experiment = project_config.get_experiment_from_id(experiment_id)
Expand Down
4 changes: 0 additions & 4 deletions optimizely/entities.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,6 @@ def __init__(
variations: list[VariationDict],
trafficAllocation: list[TrafficAllocation],
audienceIds: list[str],
includedFlags: Optional[list[str]] = None,
excludedFlags: Optional[list[str]] = None,
audienceConditions: Optional[Sequence[str | list[str]]] = None,
**kwargs: Any
):
Expand All @@ -234,8 +232,6 @@ def __init__(
self.trafficAllocation = trafficAllocation
self.audienceIds = audienceIds
self.audienceConditions = audienceConditions
self.includedFlags = includedFlags or []
self.excludedFlags = excludedFlags or []

def get_audience_conditions_or_ids(self) -> Sequence[str | list[str]]:
"""Returns audienceConditions if present, otherwise audienceIds.
Expand Down
2 changes: 0 additions & 2 deletions optimizely/helpers/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,5 +130,3 @@ class HoldoutDict(ExperimentDict):
Extends ExperimentDict with holdout-specific properties.
"""
holdoutStatus: HoldoutStatus
includedFlags: list[str]
excludedFlags: list[str]
55 changes: 1 addition & 54 deletions optimizely/project_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,44 +93,20 @@ def __init__(self, datafile: str | bytes, logger: Logger, error_handler: Any):
holdouts_data: list[types.HoldoutDict] = config.get('holdouts', [])
self.holdouts: list[entities.Holdout] = []
self.holdout_id_map: dict[str, entities.Holdout] = {}
self.global_holdouts: list[entities.Holdout] = []
self.included_holdouts: dict[str, list[entities.Holdout]] = {}
self.excluded_holdouts: dict[str, list[entities.Holdout]] = {}
self.flag_holdouts_map: dict[str, list[entities.Holdout]] = {}

# Convert holdout dicts to Holdout entities
for holdout_data in holdouts_data:
# Create Holdout entity
holdout = entities.Holdout(**holdout_data)
self.holdouts.append(holdout)

# Only process Running holdouts but doing it here for efficiency like the original Python implementation)
# Only process Running holdouts
if not holdout.is_activated:
continue

# Map by ID for quick lookup
self.holdout_id_map[holdout.id] = holdout

# Categorize as global vs flag-specific
# Global holdouts: apply to all flags unless explicitly excluded
# Flag-specific holdouts: only apply to explicitly included flags
if not holdout.includedFlags:
# This is a global holdout
self.global_holdouts.append(holdout)

# Track which flags this global holdout excludes
if holdout.excludedFlags:
for flag_id in holdout.excludedFlags:
if flag_id not in self.excluded_holdouts:
self.excluded_holdouts[flag_id] = []
self.excluded_holdouts[flag_id].append(holdout)
else:
# This holdout applies to specific flags only
for flag_id in holdout.includedFlags:
if flag_id not in self.included_holdouts:
self.included_holdouts[flag_id] = []
self.included_holdouts[flag_id].append(holdout)

# Utility maps for quick lookup
self.group_id_map: dict[str, entities.Group] = self._generate_key_map(self.groups, 'id', entities.Group)
self.experiment_id_map: dict[str, entities.Experiment] = self._generate_key_map(
Expand Down Expand Up @@ -263,22 +239,6 @@ def __init__(self, datafile: str | bytes, logger: Logger, error_handler: Any):
everyone_else_variation.variables, 'id', entities.Variation.VariableUsage
)

flag_id = feature.id
applicable_holdouts: list[entities.Holdout] = []

# Add global holdouts first, excluding any that are explicitly excluded for this flag
excluded_holdouts = self.excluded_holdouts.get(flag_id, [])
for holdout in self.global_holdouts:
if holdout not in excluded_holdouts:
applicable_holdouts.append(holdout)

# Add flag-specific local holdouts AFTER global holdouts
if flag_id in self.included_holdouts:
applicable_holdouts.extend(self.included_holdouts[flag_id])

if applicable_holdouts:
self.flag_holdouts_map[feature.key] = applicable_holdouts

rollout = None if len(feature.rolloutId) == 0 else self.rollout_id_map[feature.rolloutId]
if rollout:
for exp in rollout.experiments:
Expand Down Expand Up @@ -912,19 +872,6 @@ def get_flag_variation(

return None

def get_holdouts_for_flag(self, flag_key: str) -> list[entities.Holdout]:
""" Helper method to get holdouts from an applied feature flag.

Args:
flag_key: Key of the feature flag.

Returns:
The holdouts that apply for a specific flag as Holdout entity objects.
"""
if not self.holdouts:
return []

return self.flag_holdouts_map.get(flag_key, [])

def get_holdout(self, holdout_id: str) -> Optional[entities.Holdout]:
""" Helper method to get holdout from holdout ID.
Expand Down
4 changes: 0 additions & 4 deletions tests/test_bucketing_holdout.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,6 @@ def setUp(self):
'id': 'holdout_1',
'key': 'test_holdout',
'status': 'Running',
'includedFlags': [],
'excludedFlags': [],
'audienceIds': [],
'variations': [
{
Expand Down Expand Up @@ -92,8 +90,6 @@ def setUp(self):
'id': 'holdout_empty_1',
'key': 'empty_holdout',
'status': 'Running',
'includedFlags': [],
'excludedFlags': [],
'audienceIds': [],
'variations': [],
'trafficAllocation': []
Expand Down
61 changes: 3 additions & 58 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -1395,85 +1395,30 @@ def setUp(self):
'status': 'Running',
'variations': [],
'trafficAllocation': [],
'audienceIds': [],
'includedFlags': [],
'excludedFlags': [boolean_feature_id]
'audienceIds': []
},
{
'id': 'holdout_2',
'key': 'specific_holdout',
'status': 'Running',
'variations': [],
'trafficAllocation': [],
'audienceIds': [],
'includedFlags': [multi_variate_feature_id],
'excludedFlags': []
'audienceIds': []
},
{
'id': 'holdout_3',
'key': 'inactive_holdout',
'status': 'Inactive',
'variations': [],
'trafficAllocation': [],
'audienceIds': [],
'includedFlags': [boolean_feature_id],
'excludedFlags': []
'audienceIds': []
}
]

self.config_json_with_holdouts = json.dumps(config_body_with_holdouts)
opt_obj = optimizely.Optimizely(self.config_json_with_holdouts)
self.config_with_holdouts = opt_obj.config_manager.get_config()

def test_get_holdouts_for_flag__non_existent_flag(self):
""" Test that get_holdouts_for_flag returns empty array for non-existent flag. """

holdouts = self.config_with_holdouts.get_holdouts_for_flag('non_existent_flag')
self.assertEqual([], holdouts)

def test_get_holdouts_for_flag__returns_global_and_specific_holdouts(self):
""" Test that get_holdouts_for_flag returns global holdouts that do not exclude the flag
and specific holdouts that include the flag. """

holdouts = self.config_with_holdouts.get_holdouts_for_flag('test_feature_in_experiment_and_rollout')
self.assertEqual(2, len(holdouts))

global_holdout = next((h for h in holdouts if h['key'] == 'global_holdout'), None)
self.assertIsNotNone(global_holdout)
self.assertEqual('holdout_1', global_holdout['id'])

specific_holdout = next((h for h in holdouts if h['key'] == 'specific_holdout'), None)
self.assertIsNotNone(specific_holdout)
self.assertEqual('holdout_2', specific_holdout['id'])

def test_get_holdouts_for_flag__excludes_global_holdouts_for_excluded_flags(self):
""" Test that get_holdouts_for_flag does not return global holdouts that exclude the flag. """

holdouts = self.config_with_holdouts.get_holdouts_for_flag('boolean_single_variable_feature')
self.assertEqual(0, len(holdouts))

global_holdout = next((h for h in holdouts if h['key'] == 'global_holdout'), None)
self.assertIsNone(global_holdout)

def test_get_holdouts_for_flag__caches_results(self):
""" Test that get_holdouts_for_flag caches results for subsequent calls. """

holdouts1 = self.config_with_holdouts.get_holdouts_for_flag('test_feature_in_experiment_and_rollout')
holdouts2 = self.config_with_holdouts.get_holdouts_for_flag('test_feature_in_experiment_and_rollout')

# Should be the same object (cached)
self.assertIs(holdouts1, holdouts2)
self.assertEqual(2, len(holdouts1))

def test_get_holdouts_for_flag__returns_only_global_for_non_targeted_flags(self):
""" Test that get_holdouts_for_flag returns only global holdouts for flags not specifically targeted. """

holdouts = self.config_with_holdouts.get_holdouts_for_flag('test_feature_in_rollout')

# Should only include global holdout (not excluded and no specific targeting)
self.assertEqual(1, len(holdouts))
self.assertEqual('global_holdout', holdouts[0]['key'])

def test_get_holdout__returns_holdout_for_valid_id(self):
""" Test that get_holdout returns holdout when valid ID is provided. """

Expand Down
Loading
Loading