Skip to content

Commit 77d0cd6

Browse files
Mat001claude
andcommitted
[FSSDK-12368] Remove legacy flag-level holdout fields
Remove deprecated flag-level holdout functionality as cleanup after local holdouts (FSSDK-12369) implementation. Changes: - Removed includedFlags and excludedFlags from Holdout entity - Removed includedFlags and excludedFlags from HoldoutDict type - Removed get_holdouts_for_flag() method from ProjectConfig - Removed flag-level holdout infrastructure: - global_holdouts list - included_holdouts dict - excluded_holdouts dict - flag_holdouts_map dict - Flag-level population logic in ProjectConfig.__init__ - Removed flag-level holdout checking from DecisionService.get_variation_for_feature() - Removed 10 test methods testing removed functionality - Removed includedFlags/excludedFlags from all test fixtures Impact: 7 files modified, 310 lines deleted, 5 lines added (net: -305 lines) Verification: - grep for "includedFlags", "excludedFlags", "getHoldoutsForFlag" returns zero results - grep for "global_holdouts", "included_holdouts", "excluded_holdouts", "flag_holdouts_map" returns zero results Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent f02b3ce commit 77d0cd6

7 files changed

Lines changed: 5 additions & 310 deletions

File tree

optimizely/decision_service.py

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -733,30 +733,7 @@ def get_decision_for_flag(
733733
reasons = decide_reasons.copy() if decide_reasons else []
734734
user_id = user_context.user_id
735735

736-
# Check holdouts
737-
holdouts = project_config.get_holdouts_for_flag(feature_flag.key)
738-
for holdout in holdouts:
739-
holdout_decision = self.get_variation_for_holdout(holdout, user_context, project_config)
740-
reasons.extend(holdout_decision['reasons'])
741-
742-
decision = holdout_decision['decision']
743-
# Check if user was bucketed into holdout (has a variation)
744-
if decision.variation is None:
745-
continue
746-
747-
message = (
748-
f"The user '{user_id}' is bucketed into holdout '{holdout.key}' "
749-
f"for feature flag '{feature_flag.key}'."
750-
)
751-
self.logger.info(message)
752-
reasons.append(message)
753-
return {
754-
'decision': holdout_decision['decision'],
755-
'error': False,
756-
'reasons': reasons
757-
}
758-
759-
# If no holdout decision, check experiments then rollouts
736+
# Check experiments then rollouts
760737
if feature_flag.experimentIds:
761738
for experiment_id in feature_flag.experimentIds:
762739
experiment = project_config.get_experiment_from_id(experiment_id)

optimizely/entities.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -222,8 +222,6 @@ def __init__(
222222
variations: list[VariationDict],
223223
trafficAllocation: list[TrafficAllocation],
224224
audienceIds: list[str],
225-
includedFlags: Optional[list[str]] = None,
226-
excludedFlags: Optional[list[str]] = None,
227225
audienceConditions: Optional[Sequence[str | list[str]]] = None,
228226
**kwargs: Any
229227
):
@@ -234,8 +232,6 @@ def __init__(
234232
self.trafficAllocation = trafficAllocation
235233
self.audienceIds = audienceIds
236234
self.audienceConditions = audienceConditions
237-
self.includedFlags = includedFlags or []
238-
self.excludedFlags = excludedFlags or []
239235

240236
def get_audience_conditions_or_ids(self) -> Sequence[str | list[str]]:
241237
"""Returns audienceConditions if present, otherwise audienceIds.

optimizely/helpers/types.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,5 +130,3 @@ class HoldoutDict(ExperimentDict):
130130
Extends ExperimentDict with holdout-specific properties.
131131
"""
132132
holdoutStatus: HoldoutStatus
133-
includedFlags: list[str]
134-
excludedFlags: list[str]

optimizely/project_config.py

Lines changed: 1 addition & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -93,44 +93,20 @@ def __init__(self, datafile: str | bytes, logger: Logger, error_handler: Any):
9393
holdouts_data: list[types.HoldoutDict] = config.get('holdouts', [])
9494
self.holdouts: list[entities.Holdout] = []
9595
self.holdout_id_map: dict[str, entities.Holdout] = {}
96-
self.global_holdouts: list[entities.Holdout] = []
97-
self.included_holdouts: dict[str, list[entities.Holdout]] = {}
98-
self.excluded_holdouts: dict[str, list[entities.Holdout]] = {}
99-
self.flag_holdouts_map: dict[str, list[entities.Holdout]] = {}
10096

10197
# Convert holdout dicts to Holdout entities
10298
for holdout_data in holdouts_data:
10399
# Create Holdout entity
104100
holdout = entities.Holdout(**holdout_data)
105101
self.holdouts.append(holdout)
106102

107-
# Only process Running holdouts but doing it here for efficiency like the original Python implementation)
103+
# Only process Running holdouts
108104
if not holdout.is_activated:
109105
continue
110106

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

114-
# Categorize as global vs flag-specific
115-
# Global holdouts: apply to all flags unless explicitly excluded
116-
# Flag-specific holdouts: only apply to explicitly included flags
117-
if not holdout.includedFlags:
118-
# This is a global holdout
119-
self.global_holdouts.append(holdout)
120-
121-
# Track which flags this global holdout excludes
122-
if holdout.excludedFlags:
123-
for flag_id in holdout.excludedFlags:
124-
if flag_id not in self.excluded_holdouts:
125-
self.excluded_holdouts[flag_id] = []
126-
self.excluded_holdouts[flag_id].append(holdout)
127-
else:
128-
# This holdout applies to specific flags only
129-
for flag_id in holdout.includedFlags:
130-
if flag_id not in self.included_holdouts:
131-
self.included_holdouts[flag_id] = []
132-
self.included_holdouts[flag_id].append(holdout)
133-
134110
# Utility maps for quick lookup
135111
self.group_id_map: dict[str, entities.Group] = self._generate_key_map(self.groups, 'id', entities.Group)
136112
self.experiment_id_map: dict[str, entities.Experiment] = self._generate_key_map(
@@ -263,22 +239,6 @@ def __init__(self, datafile: str | bytes, logger: Logger, error_handler: Any):
263239
everyone_else_variation.variables, 'id', entities.Variation.VariableUsage
264240
)
265241

266-
flag_id = feature.id
267-
applicable_holdouts: list[entities.Holdout] = []
268-
269-
# Add global holdouts first, excluding any that are explicitly excluded for this flag
270-
excluded_holdouts = self.excluded_holdouts.get(flag_id, [])
271-
for holdout in self.global_holdouts:
272-
if holdout not in excluded_holdouts:
273-
applicable_holdouts.append(holdout)
274-
275-
# Add flag-specific local holdouts AFTER global holdouts
276-
if flag_id in self.included_holdouts:
277-
applicable_holdouts.extend(self.included_holdouts[flag_id])
278-
279-
if applicable_holdouts:
280-
self.flag_holdouts_map[feature.key] = applicable_holdouts
281-
282242
rollout = None if len(feature.rolloutId) == 0 else self.rollout_id_map[feature.rolloutId]
283243
if rollout:
284244
for exp in rollout.experiments:
@@ -912,20 +872,6 @@ def get_flag_variation(
912872

913873
return None
914874

915-
def get_holdouts_for_flag(self, flag_key: str) -> list[entities.Holdout]:
916-
""" Helper method to get holdouts from an applied feature flag.
917-
918-
Args:
919-
flag_key: Key of the feature flag.
920-
921-
Returns:
922-
The holdouts that apply for a specific flag as Holdout entity objects.
923-
"""
924-
if not self.holdouts:
925-
return []
926-
927-
return self.flag_holdouts_map.get(flag_key, [])
928-
929875
def get_holdout(self, holdout_id: str) -> Optional[entities.Holdout]:
930876
""" Helper method to get holdout from holdout ID.
931877

tests/test_bucketing_holdout.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,6 @@ def setUp(self):
6262
'id': 'holdout_1',
6363
'key': 'test_holdout',
6464
'status': 'Running',
65-
'includedFlags': [],
66-
'excludedFlags': [],
6765
'audienceIds': [],
6866
'variations': [
6967
{
@@ -92,8 +90,6 @@ def setUp(self):
9290
'id': 'holdout_empty_1',
9391
'key': 'empty_holdout',
9492
'status': 'Running',
95-
'includedFlags': [],
96-
'excludedFlags': [],
9793
'audienceIds': [],
9894
'variations': [],
9995
'trafficAllocation': []

tests/test_config.py

Lines changed: 3 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -1384,96 +1384,37 @@ def setUp(self):
13841384
# Create config with holdouts
13851385
config_body_with_holdouts = self.config_dict_with_features.copy()
13861386

1387-
# Use correct feature flag IDs from the datafile
1388-
boolean_feature_id = '91111' # boolean_single_variable_feature
1389-
multi_variate_feature_id = '91114' # test_feature_in_experiment_and_rollout
1390-
13911387
config_body_with_holdouts['holdouts'] = [
13921388
{
13931389
'id': 'holdout_1',
13941390
'key': 'global_holdout',
13951391
'status': 'Running',
13961392
'variations': [],
13971393
'trafficAllocation': [],
1398-
'audienceIds': [],
1399-
'includedFlags': [],
1400-
'excludedFlags': [boolean_feature_id]
1394+
'audienceIds': []
14011395
},
14021396
{
14031397
'id': 'holdout_2',
14041398
'key': 'specific_holdout',
14051399
'status': 'Running',
14061400
'variations': [],
14071401
'trafficAllocation': [],
1408-
'audienceIds': [],
1409-
'includedFlags': [multi_variate_feature_id],
1410-
'excludedFlags': []
1402+
'audienceIds': []
14111403
},
14121404
{
14131405
'id': 'holdout_3',
14141406
'key': 'inactive_holdout',
14151407
'status': 'Inactive',
14161408
'variations': [],
14171409
'trafficAllocation': [],
1418-
'audienceIds': [],
1419-
'includedFlags': [boolean_feature_id],
1420-
'excludedFlags': []
1410+
'audienceIds': []
14211411
}
14221412
]
14231413

14241414
self.config_json_with_holdouts = json.dumps(config_body_with_holdouts)
14251415
opt_obj = optimizely.Optimizely(self.config_json_with_holdouts)
14261416
self.config_with_holdouts = opt_obj.config_manager.get_config()
14271417

1428-
def test_get_holdouts_for_flag__non_existent_flag(self):
1429-
""" Test that get_holdouts_for_flag returns empty array for non-existent flag. """
1430-
1431-
holdouts = self.config_with_holdouts.get_holdouts_for_flag('non_existent_flag')
1432-
self.assertEqual([], holdouts)
1433-
1434-
def test_get_holdouts_for_flag__returns_global_and_specific_holdouts(self):
1435-
""" Test that get_holdouts_for_flag returns global holdouts that do not exclude the flag
1436-
and specific holdouts that include the flag. """
1437-
1438-
holdouts = self.config_with_holdouts.get_holdouts_for_flag('test_feature_in_experiment_and_rollout')
1439-
self.assertEqual(2, len(holdouts))
1440-
1441-
global_holdout = next((h for h in holdouts if h['key'] == 'global_holdout'), None)
1442-
self.assertIsNotNone(global_holdout)
1443-
self.assertEqual('holdout_1', global_holdout['id'])
1444-
1445-
specific_holdout = next((h for h in holdouts if h['key'] == 'specific_holdout'), None)
1446-
self.assertIsNotNone(specific_holdout)
1447-
self.assertEqual('holdout_2', specific_holdout['id'])
1448-
1449-
def test_get_holdouts_for_flag__excludes_global_holdouts_for_excluded_flags(self):
1450-
""" Test that get_holdouts_for_flag does not return global holdouts that exclude the flag. """
1451-
1452-
holdouts = self.config_with_holdouts.get_holdouts_for_flag('boolean_single_variable_feature')
1453-
self.assertEqual(0, len(holdouts))
1454-
1455-
global_holdout = next((h for h in holdouts if h['key'] == 'global_holdout'), None)
1456-
self.assertIsNone(global_holdout)
1457-
1458-
def test_get_holdouts_for_flag__caches_results(self):
1459-
""" Test that get_holdouts_for_flag caches results for subsequent calls. """
1460-
1461-
holdouts1 = self.config_with_holdouts.get_holdouts_for_flag('test_feature_in_experiment_and_rollout')
1462-
holdouts2 = self.config_with_holdouts.get_holdouts_for_flag('test_feature_in_experiment_and_rollout')
1463-
1464-
# Should be the same object (cached)
1465-
self.assertIs(holdouts1, holdouts2)
1466-
self.assertEqual(2, len(holdouts1))
1467-
1468-
def test_get_holdouts_for_flag__returns_only_global_for_non_targeted_flags(self):
1469-
""" Test that get_holdouts_for_flag returns only global holdouts for flags not specifically targeted. """
1470-
1471-
holdouts = self.config_with_holdouts.get_holdouts_for_flag('test_feature_in_rollout')
1472-
1473-
# Should only include global holdout (not excluded and no specific targeting)
1474-
self.assertEqual(1, len(holdouts))
1475-
self.assertEqual('global_holdout', holdouts[0]['key'])
1476-
14771418
def test_get_holdout__returns_holdout_for_valid_id(self):
14781419
""" Test that get_holdout returns holdout when valid ID is provided. """
14791420

@@ -1516,36 +1457,6 @@ def test_get_holdout__does_not_log_when_found(self):
15161457
self.assertIsNotNone(result)
15171458
mock_logger.error.assert_not_called()
15181459

1519-
def test_holdout_initialization__categorizes_holdouts_properly(self):
1520-
""" Test that holdouts are properly categorized during initialization. """
1521-
1522-
self.assertIn('holdout_1', self.config_with_holdouts.holdout_id_map)
1523-
self.assertIn('holdout_2', self.config_with_holdouts.holdout_id_map)
1524-
# Check if a holdout with ID 'holdout_1' exists in global_holdouts
1525-
holdout_ids_in_global = [h.id for h in self.config_with_holdouts.global_holdouts]
1526-
self.assertIn('holdout_1', holdout_ids_in_global)
1527-
1528-
# Use correct feature flag IDs
1529-
boolean_feature_id = '91111'
1530-
multi_variate_feature_id = '91114'
1531-
1532-
self.assertIn(multi_variate_feature_id, self.config_with_holdouts.included_holdouts)
1533-
self.assertTrue(len(self.config_with_holdouts.included_holdouts[multi_variate_feature_id]) > 0)
1534-
self.assertNotIn(boolean_feature_id, self.config_with_holdouts.included_holdouts)
1535-
1536-
self.assertIn(boolean_feature_id, self.config_with_holdouts.excluded_holdouts)
1537-
self.assertTrue(len(self.config_with_holdouts.excluded_holdouts[boolean_feature_id]) > 0)
1538-
1539-
def test_holdout_initialization__only_processes_running_holdouts(self):
1540-
""" Test that only running holdouts are processed during initialization. """
1541-
1542-
self.assertNotIn('holdout_3', self.config_with_holdouts.holdout_id_map)
1543-
self.assertNotIn('holdout_3', self.config_with_holdouts.global_holdouts)
1544-
1545-
boolean_feature_id = '91111'
1546-
included_for_boolean = self.config_with_holdouts.included_holdouts.get(boolean_feature_id)
1547-
self.assertIsNone(included_for_boolean)
1548-
15491460

15501461
class FeatureRolloutConfigTest(base.BaseTest):
15511462
"""Tests for Feature Rollout support in ProjectConfig parsing."""

0 commit comments

Comments
 (0)