Skip to content

Commit c05c09b

Browse files
Mat001claudeFarhanAnjum-opti
authored
[AI-FSSDK] [FSSDK-12368] Cleanup flag base setup (local holdouts) (#507)
* [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> * [FSSDK-12368] Restore holdout evaluation in decide flow, remove only flag-level include/exclude fields The previous cleanup removed the entire holdout evaluation from the decide flow. This restores holdout checking while keeping the removal of legacy includedFlags/excludedFlags fields — all running holdouts now apply to all flags uniformly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> Co-authored-by: Farhan Anjum <Farhan.Anjum@optimizely.com>
1 parent f02b3ce commit c05c09b

6 files changed

Lines changed: 6 additions & 276 deletions

File tree

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: 3 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,6 @@ 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]] = {}
9996
self.flag_holdouts_map: dict[str, list[entities.Holdout]] = {}
10097

10198
# Convert holdout dicts to Holdout entities
@@ -104,33 +101,13 @@ def __init__(self, datafile: str | bytes, logger: Logger, error_handler: Any):
104101
holdout = entities.Holdout(**holdout_data)
105102
self.holdouts.append(holdout)
106103

107-
# Only process Running holdouts but doing it here for efficiency like the original Python implementation)
104+
# Only process Running holdouts
108105
if not holdout.is_activated:
109106
continue
110107

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

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-
134111
# Utility maps for quick lookup
135112
self.group_id_map: dict[str, entities.Group] = self._generate_key_map(self.groups, 'id', entities.Group)
136113
self.experiment_id_map: dict[str, entities.Experiment] = self._generate_key_map(
@@ -263,19 +240,8 @@ def __init__(self, datafile: str | bytes, logger: Logger, error_handler: Any):
263240
everyone_else_variation.variables, 'id', entities.Variation.VariableUsage
264241
)
265242

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-
243+
# Map all running holdouts to this flag
244+
applicable_holdouts = list(self.holdout_id_map.values())
279245
if applicable_holdouts:
280246
self.flag_holdouts_map[feature.key] = applicable_holdouts
281247

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)