Skip to content

Commit 4470ca9

Browse files
ozayr-zaviarozayr-zaviar
authored
fix: decision service for group and multiple experiments. (#322)
1 parent 46651fa commit 4470ca9

File tree

7 files changed

+631
-130
lines changed

7 files changed

+631
-130
lines changed

‎optimizely/decision_service.py‎

Lines changed: 7 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -413,39 +413,6 @@ def get_variation_for_rollout(self, project_config, rollout, user_id, attributes
413413

414414
return Decision(None, None, enums.DecisionSources.ROLLOUT), decide_reasons
415415

416-
def get_experiment_in_group(self, project_config, group, bucketing_id):
417-
""" Determine which experiment in the group the user is bucketed into.
418-
419-
Args:
420-
project_config: Instance of ProjectConfig.
421-
group: The group to bucket the user into.
422-
bucketing_id: ID to be used for bucketing the user.
423-
424-
Returns:
425-
Experiment if the user is bucketed into an experiment in the specified group. None otherwise
426-
and array of log messages representing decision making.
427-
"""
428-
decide_reasons = []
429-
experiment_id = self.bucketer.find_bucket(
430-
project_config, bucketing_id, group.id, group.trafficAllocation)
431-
if experiment_id:
432-
experiment = project_config.get_experiment_from_id(experiment_id)
433-
if experiment:
434-
message = 'User with bucketing ID "%s" is in experiment %s of group %s.' % \
435-
(bucketing_id, experiment.key, group.id)
436-
self.logger.info(
437-
message
438-
)
439-
decide_reasons.append(message)
440-
return experiment, decide_reasons
441-
message = 'User with bucketing ID "%s" is not in any experiments of group %s.' % (bucketing_id, group.id)
442-
self.logger.info(
443-
message
444-
)
445-
decide_reasons.append(message)
446-
447-
return None, decide_reasons
448-
449416
def get_variation_for_feature(self, project_config, feature, user_id, attributes=None, ignore_user_profile=False):
450417
""" Returns the experiment/variation the user is bucketed in for the given feature.
451418
@@ -462,31 +429,18 @@ def get_variation_for_feature(self, project_config, feature, user_id, attributes
462429
decide_reasons = []
463430
bucketing_id, reasons = self._get_bucketing_id(user_id, attributes)
464431
decide_reasons += reasons
465-
# First check if the feature is in a mutex group
466-
if feature.groupId:
467-
group = project_config.get_group(feature.groupId)
468-
if group:
469-
experiment, reasons = self.get_experiment_in_group(project_config, group, bucketing_id)
470-
decide_reasons += reasons
471-
if experiment and experiment.id in feature.experimentIds:
432+
433+
# Check if the feature flag is under an experiment and the the user is bucketed into one of these experiments
434+
if feature.experimentIds:
435+
# Evaluate each experiment ID and return the first bucketed experiment variation
436+
for experiment in feature.experimentIds:
437+
experiment = project_config.get_experiment_from_id(experiment)
438+
if experiment:
472439
variation, variation_reasons = self.get_variation(
473440
project_config, experiment, user_id, attributes, ignore_user_profile)
474441
decide_reasons += variation_reasons
475442
if variation:
476443
return Decision(experiment, variation, enums.DecisionSources.FEATURE_TEST), decide_reasons
477-
else:
478-
self.logger.error(enums.Errors.INVALID_GROUP_ID.format('_get_variation_for_feature'))
479-
480-
# Next check if the feature is being experimented on
481-
elif feature.experimentIds:
482-
# If an experiment is not in a group, then the feature can only be associated with one experiment
483-
experiment = project_config.get_experiment_from_id(feature.experimentIds[0])
484-
if experiment:
485-
variation, variation_reasons = self.get_variation(
486-
project_config, experiment, user_id, attributes, ignore_user_profile)
487-
decide_reasons += variation_reasons
488-
if variation:
489-
return Decision(experiment, variation, enums.DecisionSources.FEATURE_TEST), decide_reasons
490444

491445
# Next check if user is part of a rollout
492446
if feature.rolloutId:

‎optimizely/project_config.py‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -128,13 +128,6 @@ def __init__(self, datafile, logger, error_handler):
128128
# Add this experiment in experiment-feature map.
129129
self.experiment_feature_map[exp_id] = [feature.id]
130130

131-
experiment_in_feature = self.experiment_id_map[exp_id]
132-
# Check if any of the experiments are in a group and add the group id for faster bucketing later on
133-
if experiment_in_feature.groupId:
134-
feature.groupId = experiment_in_feature.groupId
135-
# Experiments in feature can only belong to one mutex group
136-
break
137-
138131
@staticmethod
139132
def _generate_key_map(entity_list, key, entity_class):
140133
""" Helper method to generate map from key to entity object for given list of dicts.

‎tests/base.py‎

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,78 @@ def setUp(self, config_dict='config_dict'):
196196
},
197197
],
198198
},
199+
{
200+
'key': 'test_experiment3',
201+
'status': 'Running',
202+
'layerId': '6',
203+
"audienceConditions": [
204+
"or",
205+
"11160"
206+
],
207+
'audienceIds': ['11160'],
208+
'id': '111134',
209+
'forcedVariations': {},
210+
'trafficAllocation': [
211+
{'entityId': '222239', 'endOfRange': 2500},
212+
{'entityId': '', 'endOfRange': 5000},
213+
{'entityId': '', 'endOfRange': 7500},
214+
{'entityId': '', 'endOfRange': 10000}
215+
],
216+
'variations': [
217+
{
218+
'id': '222239',
219+
'key': 'control',
220+
'variables': [],
221+
}
222+
],
223+
},
224+
{
225+
'key': 'test_experiment4',
226+
'status': 'Running',
227+
'layerId': '7',
228+
"audienceConditions": [
229+
"or",
230+
"11160"
231+
],
232+
'audienceIds': ['11160'],
233+
'id': '111135',
234+
'forcedVariations': {},
235+
'trafficAllocation': [
236+
{'entityId': '222240', 'endOfRange': 5000},
237+
{'entityId': '', 'endOfRange': 7500},
238+
{'entityId': '', 'endOfRange': 10000}
239+
],
240+
'variations': [
241+
{
242+
'id': '222240',
243+
'key': 'control',
244+
'variables': [],
245+
}
246+
],
247+
},
248+
{
249+
'key': 'test_experiment5',
250+
'status': 'Running',
251+
'layerId': '8',
252+
"audienceConditions": [
253+
"or",
254+
"11160"
255+
],
256+
'audienceIds': ['11160'],
257+
'id': '111136',
258+
'forcedVariations': {},
259+
'trafficAllocation': [
260+
{'entityId': '222241', 'endOfRange': 7500},
261+
{'entityId': '', 'endOfRange': 10000}
262+
],
263+
'variations': [
264+
{
265+
'id': '222241',
266+
'key': 'control',
267+
'variables': [],
268+
}
269+
],
270+
},
199271
],
200272
'groups': [
201273
{
@@ -239,6 +311,72 @@ def setUp(self, config_dict='config_dict'):
239311
{'entityId': '32222', "endOfRange": 3000},
240312
{'entityId': '32223', 'endOfRange': 7500},
241313
],
314+
},
315+
{
316+
'id': '19229',
317+
'policy': 'random',
318+
'experiments': [
319+
{
320+
'id': '42222',
321+
'key': 'group_2_exp_1',
322+
'status': 'Running',
323+
"audienceConditions": [
324+
"or",
325+
"11160"
326+
],
327+
'audienceIds': ['11160'],
328+
'layerId': '211183',
329+
'variations': [
330+
{'key': 'var_1', 'id': '38901'},
331+
],
332+
'forcedVariations': {},
333+
'trafficAllocation': [
334+
{'entityId': '38901', 'endOfRange': 10000}
335+
],
336+
},
337+
{
338+
'id': '42223',
339+
'key': 'group_2_exp_2',
340+
'status': 'Running',
341+
"audienceConditions": [
342+
"or",
343+
"11160"
344+
],
345+
'audienceIds': ['11160'],
346+
'layerId': '211184',
347+
'variations': [
348+
{'key': 'var_1', 'id': '38905'}
349+
],
350+
'forcedVariations': {},
351+
'trafficAllocation': [
352+
{'entityId': '38905', 'endOfRange': 10000}
353+
],
354+
},
355+
{
356+
'id': '42224',
357+
'key': 'group_2_exp_3',
358+
'status': 'Running',
359+
"audienceConditions": [
360+
"or",
361+
"11160"
362+
],
363+
'audienceIds': ['11160'],
364+
'layerId': '211185',
365+
'variations': [
366+
{'key': 'var_1', 'id': '38906'}
367+
],
368+
'forcedVariations': {},
369+
'trafficAllocation': [
370+
{'entityId': '38906', 'endOfRange': 10000}
371+
],
372+
}
373+
],
374+
'trafficAllocation': [
375+
{'entityId': '42222', "endOfRange": 2500},
376+
{'entityId': '42223', 'endOfRange': 5000},
377+
{'entityId': '42224', "endOfRange": 7500},
378+
{'entityId': '', 'endOfRange': 10000},
379+
],
242380
}
243381
],
244382
'attributes': [{'key': 'test_attribute', 'id': '111094'}],
@@ -255,6 +393,12 @@ def setUp(self, config_dict='config_dict'):
255393
'{"name": "test_attribute", "type": "custom_attribute", "value": "test_value_2"}]]]',
256394
'id': '11159',
257395
},
396+
{
397+
'name': 'Test attribute users 3',
398+
'conditions': "[\"and\", [\"or\", [\"or\", {\"match\": \"exact\", \"name\": \
399+
\"experiment_attr\", \"type\": \"custom_attribute\", \"value\": \"group_experiment\"}]]]",
400+
'id': '11160',
401+
}
258402
],
259403
'rollouts': [
260404
{'id': '201111', 'experiments': []},
@@ -364,6 +508,20 @@ def setUp(self, config_dict='config_dict'):
364508
'rolloutId': '211111',
365509
'variables': [],
366510
},
511+
{
512+
'id': '91115',
513+
'key': 'test_feature_in_exclusion_group',
514+
'experimentIds': ['42222', '42223', '42224'],
515+
'rolloutId': '211111',
516+
'variables': [],
517+
},
518+
{
519+
'id': '91116',
520+
'key': 'test_feature_in_multiple_experiments',
521+
'experimentIds': ['111134', '111135', '111136'],
522+
'rolloutId': '211111',
523+
'variables': [],
524+
},
367525
],
368526
}
369527

‎tests/test_config.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -501,7 +501,7 @@ def test_init__with_v4_datafile(self):
501501
'211111',
502502
{'number_of_projects': entities.Variable('131', 'number_of_projects', 'integer', '10')},
503503
),
504-
'test_feature_in_group': entities.FeatureFlag('91113', 'test_feature_in_group', ['32222'], '', {}, '19228'),
504+
'test_feature_in_group': entities.FeatureFlag('91113', 'test_feature_in_group', ['32222'], '', {}),
505505
}
506506

507507
expected_rollout_id_map = {

0 commit comments

Comments
 (0)