Skip to content

Commit d7387ad

Browse files
[AI-FSSDK] [FSSDK-12337] Simplify feature rollout config parsing
- Inline _get_everyone_else_variation logic, remove unnecessary static method - Use get_rollout_from_id() to match TDD pseudocode - Remove isinstance check (rollout experiments are always dicts) - Remove 3 unit tests for deleted helper method (edge cases already covered by integration-level tests) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 2fba50e commit d7387ad

File tree

2 files changed

+6
-99
lines changed

2 files changed

+6
-99
lines changed

optimizely/project_config.py

Lines changed: 6 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -234,23 +234,19 @@ def __init__(self, datafile: str | bytes, logger: Logger, error_handler: Any):
234234

235235
# Feature Rollout support: inject the "everyone else" variation
236236
# into any experiment with type == "feature_rollout"
237-
rollout_for_flag = (
238-
None if len(feature.rolloutId) == 0
239-
else self.rollout_id_map.get(feature.rolloutId)
240-
)
241-
if rollout_for_flag:
242-
everyone_else_variation = self._get_everyone_else_variation(rollout_for_flag)
243-
if everyone_else_variation is not None:
237+
rollout_for_flag = self.get_rollout_from_id(feature.rolloutId) if feature.rolloutId else None
238+
if rollout_for_flag and rollout_for_flag.experiments:
239+
everyone_else_rule = rollout_for_flag.experiments[-1]
240+
everyone_else_variations = everyone_else_rule.get('variations', [])
241+
if everyone_else_variations:
242+
everyone_else_variation = everyone_else_variations[0]
244243
for experiment in rules:
245244
if getattr(experiment, 'type', None) == 'feature_rollout':
246-
# Append the everyone else variation to the experiment
247245
experiment.variations.append(everyone_else_variation)
248-
# Add traffic allocation entry with endOfRange=10000
249246
experiment.trafficAllocation.append({
250247
'entityId': everyone_else_variation['id'],
251248
'endOfRange': 10000,
252249
})
253-
# Update variation maps for this experiment
254250
var_entity = entities.Variation(
255251
id=everyone_else_variation['id'],
256252
key=everyone_else_variation['key'],
@@ -340,32 +336,6 @@ def _generate_key_map(
340336

341337
return key_map
342338

343-
@staticmethod
344-
def _get_everyone_else_variation(rollout: entities.Layer) -> Optional[types.VariationDict]:
345-
""" Get the "everyone else" variation from a rollout.
346-
347-
The "everyone else" rule is the last experiment in the rollout,
348-
and its first variation is the "everyone else" variation.
349-
350-
Args:
351-
rollout: The rollout (Layer) entity to get the variation from.
352-
353-
Returns:
354-
The "everyone else" variation dict, or None if not available.
355-
"""
356-
if not rollout.experiments:
357-
return None
358-
359-
everyone_else_rule = rollout.experiments[-1]
360-
variations = everyone_else_rule.get('variations', []) if isinstance(
361-
everyone_else_rule, dict
362-
) else getattr(everyone_else_rule, 'variations', [])
363-
364-
if not variations:
365-
return None
366-
367-
return variations[0]
368-
369339
@staticmethod
370340
def _deserialize_audience(audience_map: dict[str, entities.Audience]) -> dict[str, entities.Audience]:
371341
""" Helper method to de-serialize and populate audience map with the condition list and structure.

tests/test_config.py

Lines changed: 0 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -2363,66 +2363,3 @@ def test_existing_datafile_not_broken(self):
23632363
self.assertEqual(len(experiment.trafficAllocation), 2)
23642364
self.assertIsNone(experiment.type)
23652365

2366-
def test_get_everyone_else_variation_helper(self):
2367-
"""Test the _get_everyone_else_variation static method directly."""
2368-
layer = entities.Layer(
2369-
id='rollout_1',
2370-
experiments=[
2371-
{
2372-
'id': 'rule_1',
2373-
'key': 'rule_1',
2374-
'status': 'Running',
2375-
'forcedVariations': {},
2376-
'layerId': 'rollout_1',
2377-
'audienceIds': [],
2378-
'trafficAllocation': [],
2379-
'variations': [
2380-
{'key': 'var_1', 'id': 'var_1', 'featureEnabled': True}
2381-
],
2382-
},
2383-
{
2384-
'id': 'everyone_else',
2385-
'key': 'everyone_else',
2386-
'status': 'Running',
2387-
'forcedVariations': {},
2388-
'layerId': 'rollout_1',
2389-
'audienceIds': [],
2390-
'trafficAllocation': [],
2391-
'variations': [
2392-
{'key': 'ee_var', 'id': 'ee_var', 'featureEnabled': False}
2393-
],
2394-
},
2395-
],
2396-
)
2397-
2398-
result = ProjectConfig._get_everyone_else_variation(layer)
2399-
self.assertIsNotNone(result)
2400-
self.assertEqual(result['id'], 'ee_var')
2401-
self.assertEqual(result['key'], 'ee_var')
2402-
2403-
def test_get_everyone_else_variation_empty_rollout(self):
2404-
"""Test _get_everyone_else_variation returns None for empty rollout."""
2405-
layer = entities.Layer(id='empty_rollout', experiments=[])
2406-
result = ProjectConfig._get_everyone_else_variation(layer)
2407-
self.assertIsNone(result)
2408-
2409-
def test_get_everyone_else_variation_no_variations(self):
2410-
"""Test _get_everyone_else_variation returns None when last rule has no variations."""
2411-
layer = entities.Layer(
2412-
id='rollout_1',
2413-
experiments=[
2414-
{
2415-
'id': 'rule_1',
2416-
'key': 'rule_1',
2417-
'status': 'Running',
2418-
'forcedVariations': {},
2419-
'layerId': 'rollout_1',
2420-
'audienceIds': [],
2421-
'trafficAllocation': [],
2422-
'variations': [],
2423-
},
2424-
],
2425-
)
2426-
2427-
result = ProjectConfig._get_everyone_else_variation(layer)
2428-
self.assertIsNone(result)

0 commit comments

Comments
 (0)