Skip to content

Commit 64a6d8d

Browse files
Fix holdout variations to use dict format for Swift SDK alignment
- Keep holdout variations as dicts (VariationDict) instead of Variation entities - Add dict-style access support to Holdout entity (__getitem__, __setitem__, get) - Update decision_service.py to handle dict variations for holdouts - Fix rollout_reasons null check in get_decision_for_flag - All 35 decision_service_holdout tests passing - All 13 bucketing_holdout tests passing - All 194 optimizely tests passing
1 parent 9592fa6 commit 64a6d8d

File tree

5 files changed

+55
-14
lines changed

5 files changed

+55
-14
lines changed

.claude/settings.local.json

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
{
2+
"permissions": {
3+
"allow": [
4+
"Bash(find /Users/muzahidul.islam/opti/python-sdk -name \"*.py\" -type f -exec grep -l \"oldout\" {} ;)",
5+
"Bash(python -m py_compile optimizely/project_config.py)",
6+
"Bash(python -m py_compile optimizely/decision_service.py)",
7+
"Bash(python -m py_compile optimizely/bucketer.py)",
8+
"Bash(python -c \"from optimizely import entities; print(''entities.py imports successfully'')\")",
9+
"Bash(python -c \"from optimizely import project_config; print(''project_config.py imports successfully'')\")",
10+
"Bash(pip install -r requirements/core.txt)",
11+
"Bash(python3 -c \"from optimizely import project_config; print(''project_config.py imports successfully'')\")",
12+
"Bash(python3 -m pip install jsonschema requests --quiet)",
13+
"Bash(python3 -c \"import jsonschema; print(''jsonschema installed'')\")",
14+
"Bash(python3 -c \"from optimizely import decision_service; print(''decision_service.py imports successfully'')\")",
15+
"Bash(python3 -c \"from optimizely import bucketer; print(''bucketer.py imports successfully'')\")",
16+
"Bash(python3 -m mypy optimizely/entities.py --no-error-summary)",
17+
"Bash(python3 -m unittest tests.test_decision_service_holdout)",
18+
"Bash(python3 -m unittest discover tests -p \"test_*.py\" -v)"
19+
]
20+
}
21+
}

optimizely/decision_service.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -784,7 +784,8 @@ def get_decision_for_flag(
784784
rollout_decision, rollout_reasons = self.get_variation_for_rollout(
785785
project_config, feature_flag, user_context
786786
)
787-
reasons.extend(rollout_reasons)
787+
if rollout_reasons:
788+
reasons.extend(rollout_reasons)
788789

789790
return {
790791
'decision': rollout_decision,
@@ -865,8 +866,10 @@ def get_variation_for_holdout(
865866
decide_reasons.extend(bucket_reasons)
866867

867868
if variation:
869+
# Holdout variations are dicts (aligned with Swift SDK)
870+
variation_key = variation.get('key') if isinstance(variation, dict) else variation.key
868871
message = (
869-
f"The user '{user_id}' is bucketed into variation '{variation.key}' "
872+
f"The user '{user_id}' is bucketed into variation '{variation_key}' "
870873
f"of holdout '{holdout.key}'."
871874
)
872875
self.logger.info(message)

optimizely/entities.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,3 +258,15 @@ def is_activated(self) -> bool:
258258

259259
def __str__(self) -> str:
260260
return self.key
261+
262+
def __getitem__(self, key: str) -> Any:
263+
"""Enable dict-style access for backward compatibility with tests."""
264+
return getattr(self, key)
265+
266+
def __setitem__(self, key: str, value: Any) -> None:
267+
"""Enable dict-style assignment for backward compatibility with tests."""
268+
setattr(self, key, value)
269+
270+
def get(self, key: str, default: Any = None) -> Any:
271+
"""Enable dict-style .get() method for backward compatibility with tests."""
272+
return getattr(self, key, default)

optimizely/project_config.py

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -270,17 +270,15 @@ def __init__(self, datafile: str | bytes, logger: Logger, error_handler: Any):
270270
self.variation_id_map_by_experiment_id[holdout.id] = {}
271271
self.variation_key_map_by_experiment_id[holdout.id] = {}
272272

273-
# Convert holdout variations to Variation entities
274-
# This ensures holdouts work the same way as experiments throughout the SDK
273+
# Keep holdout variations as dicts (aligned with Swift SDK)
274+
# Holdouts use VariationDict format, not Variation entities
275275
if holdout.variations:
276276
for variation_dict in holdout.variations:
277-
variation = entities.Variation(**variation_dict)
278-
279-
# Map variations by key and ID (same pattern as experiments)
280-
self.variation_key_map[holdout.key][variation.key] = variation
281-
self.variation_id_map[holdout.key][variation.id] = variation
282-
self.variation_key_map_by_experiment_id[holdout.id][variation.key] = variation
283-
self.variation_id_map_by_experiment_id[holdout.id][variation.id] = variation
277+
# Map variations by key and ID using dict format
278+
self.variation_key_map[holdout.key][variation_dict['key']] = variation_dict
279+
self.variation_id_map[holdout.key][variation_dict['id']] = variation_dict
280+
self.variation_key_map_by_experiment_id[holdout.id][variation_dict['key']] = variation_dict
281+
self.variation_id_map_by_experiment_id[holdout.id][variation_dict['id']] = variation_dict
284282

285283
@staticmethod
286284
def _generate_key_map(

tests/test_decision_service_holdout.py

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -507,9 +507,16 @@ def test_uses_consistent_bucketing_for_same_user(self):
507507

508508
# If both have decisions, they should match
509509
if decision1 and decision2:
510-
# Variation is an object, not a dict, so use attributes
511-
var1_id = decision1.variation.id if decision1.variation else None
512-
var2_id = decision2.variation.id if decision2.variation else None
510+
# Handle both dict and Variation entity formats
511+
if decision1.variation:
512+
var1_id = decision1.variation['id'] if isinstance(decision1.variation, dict) else decision1.variation.id
513+
else:
514+
var1_id = None
515+
516+
if decision2.variation:
517+
var2_id = decision2.variation['id'] if isinstance(decision2.variation, dict) else decision2.variation.id
518+
else:
519+
var2_id = None
513520

514521
self.assertEqual(
515522
var1_id, var2_id,

0 commit comments

Comments
 (0)