Skip to content

Commit 669bca2

Browse files
aliabbasrizvialiabbasrizvi
authored
Updating evaluation of rollout rules (#72)
1 parent b76340c commit 669bca2

File tree

7 files changed

+266
-126
lines changed

7 files changed

+266
-126
lines changed

‎optimizely/decision_service.py

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -155,29 +155,55 @@ def get_variation(self, experiment, user_id, attributes, ignore_user_profile=Fal
155155

156156
return None
157157

158-
def get_variation_for_layer(self, layer, user_id, attributes=None, ignore_user_profile=False):
159-
""" Determine which variation the user is in for a given layer.
158+
def get_variation_for_rollout(self, rollout, user_id, attributes=None, ignore_user_profile=False):
159+
""" Determine which variation the user is in for a given rollout.
160160
Returns the variation of the first experiment the user qualifies for.
161161
162162
Args:
163-
layer: Layer for which we are getting the variation.
163+
rollout: Rollout for which we are getting the variation.
164164
user_id: ID for user.
165165
attributes: Dict representing user attributes.
166166
ignore_user_profile: True to ignore the user profile lookup. Defaults to False.
167167
168-
169168
Returns:
170169
Variation the user should see. None if the user is not in any of the layer's experiments.
171170
"""
171+
172172
# Go through each experiment in order and try to get the variation for the user
173-
if layer:
174-
for experiment_dict in layer.experiments:
175-
experiment = self.config.get_experiment_from_key(experiment_dict['key'])
176-
variation = self.get_variation(experiment, user_id, attributes, ignore_user_profile)
173+
if rollout and len(rollout.experiments) > 0:
174+
for idx in range(len(rollout.experiments) - 1):
175+
experiment = self.config.get_experiment_from_key(rollout.experiments[idx].get('key'))
176+
177+
# Check if user meets audience conditions for targeting rule
178+
if not audience_helper.is_user_in_experiment(self.config, experiment, attributes):
179+
self.logger.log(
180+
enums.LogLevels.DEBUG,
181+
'User "%s" does not meet conditions for targeting rule %s.' % (user_id, idx + 1)
182+
)
183+
continue
184+
185+
self.logger.log(enums.LogLevels.DEBUG, 'User "%s" meets conditions for targeting rule %s.' % (user_id, idx + 1))
186+
variation = self.bucketer.bucket(experiment, user_id)
177187
if variation:
178188
self.logger.log(enums.LogLevels.DEBUG,
179189
'User "%s" is in variation %s of experiment %s.' % (user_id, variation.key, experiment.key))
180-
# Return as soon as we get a variation
190+
return variation
191+
else:
192+
# Evaluate no further rules
193+
self.logger.log(enums.LogLevels.DEBUG,
194+
'User "%s" is not in the traffic group for the targeting else. '
195+
'Checking "Everyone Else" rule now.' % user_id)
196+
break
197+
198+
# Evaluate last rule i.e. "Everyone Else" rule
199+
everyone_else_experiment = rollout.experiments[-1]
200+
if audience_helper.is_user_in_experiment(self.config,
201+
self.config.get_experiment_from_key(rollout.experiments[-1].get('key')),
202+
attributes):
203+
variation = self.bucketer.bucket(everyone_else_experiment, user_id)
204+
if variation:
205+
self.logger.log(enums.LogLevels.DEBUG,
206+
'User "%s" meets conditions for targeting rule "Everyone Else".' % user_id)
181207
return variation
182208

183209
return None
@@ -193,6 +219,7 @@ def get_variation_for_feature(self, feature, user_id, attributes=None):
193219
Returns:
194220
Variation that the user is bucketed in. None if the user is not in any variation.
195221
"""
222+
196223
variation = None
197224

198225
# First check if the feature is in a mutex group
@@ -223,7 +250,7 @@ def get_variation_for_feature(self, feature, user_id, attributes=None):
223250
# Next check if user is part of a rollout
224251
if not variation and feature.rolloutId:
225252
rollout = self.config.get_layer_from_id(feature.rolloutId)
226-
variation = self.get_variation_for_layer(rollout, user_id, attributes, ignore_user_profile=True)
253+
variation = self.get_variation_for_rollout(rollout, user_id, attributes, ignore_user_profile=True)
227254

228255
return variation
229256

‎tests/base.py

Lines changed: 89 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,15 @@ def setUp(self):
131131
'id': '111094'
132132
}],
133133
'audiences': [{
134-
'name': 'Test attribute users',
134+
'name': 'Test attribute users 1',
135135
'conditions': '["and", ["or", ["or", '
136-
'{"name": "test_attribute", "type": "custom_attribute", "value": "test_value"}]]]',
136+
'{"name": "test_attribute", "type": "custom_attribute", "value": "test_value_1"}]]]',
137137
'id': '11154'
138+
}, {
139+
'name': 'Test attribute users 2',
140+
'conditions': '["and", ["or", ["or", '
141+
'{"name": "test_attribute", "type": "custom_attribute", "value": "test_value_2"}]]]',
142+
'id': '11159'
138143
}],
139144
'projectId': '111001'
140145
}
@@ -242,10 +247,15 @@ def setUp(self):
242247
'id': '111094'
243248
}],
244249
'audiences': [{
245-
'name': 'Test attribute users',
250+
'name': 'Test attribute users 1',
246251
'conditions': '["and", ["or", ["or", '
247-
'{"name": "test_attribute", "type": "custom_attribute", "value": "test_value"}]]]',
252+
'{"name": "test_attribute", "type": "custom_attribute", "value": "test_value_1"}]]]',
248253
'id': '11154'
254+
}, {
255+
'name': 'Test attribute users 2',
256+
'conditions': '["and", ["or", ["or", '
257+
'{"name": "test_attribute", "type": "custom_attribute", "value": "test_value_2"}]]]',
258+
'id': '11159'
249259
}],
250260
'rollouts': [{
251261
'id': '211111',
@@ -257,24 +267,48 @@ def setUp(self):
257267
'layerId': '211111',
258268
'audienceIds': ['11154'],
259269
'trafficAllocation': [{
260-
'entityId': '211128',
261-
'endOfRange': 5000
262-
}, {
263270
'entityId': '211129',
264271
'endOfRange': 9000
265272
}],
266273
'variations': [{
267-
'key': '211128',
268-
'id': '211128'
269-
}, {
270274
'key': '211129',
271275
'id': '211129'
272276
}]
277+
}, {
278+
'id': '211137',
279+
'key': '211137',
280+
'status': 'Running',
281+
'forcedVariations': {},
282+
'layerId': '211111',
283+
'audienceIds': ['11159'],
284+
'trafficAllocation': [{
285+
'entityId': '211139',
286+
'endOfRange': 3000
287+
}],
288+
'variations': [{
289+
'key': '211139',
290+
'id': '211139'
291+
}]
292+
}, {
293+
'id': '211147',
294+
'key': '211147',
295+
'status': 'Running',
296+
'forcedVariations': {},
297+
'layerId': '211111',
298+
'audienceIds': [],
299+
'trafficAllocation': [{
300+
'entityId': '211149',
301+
'endOfRange': 6000
302+
}],
303+
'variations': [{
304+
'key': '211149',
305+
'id': '211149'
306+
}]
273307
}]
274308
}],
275309
'featureFlags': [{
276310
'id': '91111',
277-
'key': 'test_feature_1',
311+
'key': 'test_feature_in_experiment',
278312
'experimentIds': ['111127'],
279313
'rolloutId': '',
280314
'variables': [{
@@ -290,7 +324,7 @@ def setUp(self):
290324
}]
291325
}, {
292326
'id': '91112',
293-
'key': 'test_feature_2',
327+
'key': 'test_feature_in_rollout',
294328
'experimentIds': [],
295329
'rolloutId': '211111',
296330
'variables': [],
@@ -424,8 +458,13 @@ def setUp(self):
424458
'audiences': [{
425459
'name': 'Test attribute users',
426460
'conditions': '["and", ["or", ["or", '
427-
'{"name": "test_attribute", "type": "custom_attribute", "value": "test_value"}]]]',
461+
'{"name": "test_attribute", "type": "custom_attribute", "value": "test_value_1"}]]]',
428462
'id': '11154'
463+
}, {
464+
'name': 'Test attribute users',
465+
'conditions': '["and", ["or", ["or", '
466+
'{"name": "test_attribute", "type": "custom_attribute", "value": "test_value_2"}]]]',
467+
'id': '11159'
429468
}],
430469
'projectId': '111001'
431470
}
@@ -474,10 +513,15 @@ def setUp(self):
474513
'id': '111094'
475514
}],
476515
'audiences': [{
477-
'name': 'Test attribute users',
516+
'name': 'Test attribute users 1',
478517
'conditions': '["and", ["or", ["or", '
479-
'{"name": "test_attribute", "type": "custom_attribute", "value": "test_value"}]]]',
518+
'{"name": "test_attribute", "type": "custom_attribute", "value": "test_value_1"}]]]',
480519
'id': '11154'
520+
}, {
521+
'name': 'Test attribute users 2',
522+
'conditions': '["and", ["or", ["or", '
523+
'{"name": "test_attribute", "type": "custom_attribute", "value": "test_value_2"}]]]',
524+
'id': '11159'
481525
}],
482526
'rollouts': [{
483527
'id': '211111',
@@ -489,19 +533,43 @@ def setUp(self):
489533
'layerId': '211111',
490534
'audienceIds': ['11154'],
491535
'trafficAllocation': [{
492-
'entityId': '211128',
493-
'endOfRange': 5000
494-
}, {
495536
'entityId': '211129',
496537
'endOfRange': 9000
497538
}],
498539
'variations': [{
499-
'key': '211128',
500-
'id': '211128'
501-
}, {
502540
'key': '211129',
503541
'id': '211129'
504542
}]
543+
}, {
544+
'id': '211137',
545+
'key': '211137',
546+
'status': 'Running',
547+
'forcedVariations': {},
548+
'layerId': '211111',
549+
'audienceIds': ['11159'],
550+
'trafficAllocation': [{
551+
'entityId': '211139',
552+
'endOfRange': 3000
553+
}],
554+
'variations': [{
555+
'key': '211139',
556+
'id': '211139'
557+
}]
558+
}, {
559+
'id': '211147',
560+
'key': '211147',
561+
'status': 'Running',
562+
'forcedVariations': {},
563+
'layerId': '211111',
564+
'audienceIds': [],
565+
'trafficAllocation': [{
566+
'entityId': '211149',
567+
'endOfRange': 6000
568+
}],
569+
'variations': [{
570+
'key': '211149',
571+
'id': '211149'
572+
}]
505573
}]
506574
}],
507575
'featureFlags': [{

‎tests/helpers_tests/test_audience.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ def test_is_match__audience_condition_matches(self):
2323
""" Test that is_match returns True when audience conditions are met. """
2424

2525
user_attributes = {
26-
'test_attribute': 'test_value',
26+
'test_attribute': 'test_value_1',
2727
'browser_type': 'firefox',
2828
'location': 'San Francisco'
2929
}
@@ -45,7 +45,7 @@ def test_is_user_in_experiment__no_audience(self):
4545
""" Test that is_user_in_experiment returns True when experiment is using no audience. """
4646

4747
user_attributes = {
48-
'test_attribute': 'test_value',
48+
'test_attribute': 'test_value_1',
4949
'browser_type': 'firefox',
5050
'location': 'San Francisco'
5151
}
@@ -58,7 +58,7 @@ def test_is_user_in_experiment__audience_conditions_are_met(self):
5858
""" Test that is_user_in_experiment returns True when audience conditions are met. """
5959

6060
user_attributes = {
61-
'test_attribute': 'test_value',
61+
'test_attribute': 'test_value_1',
6262
'browser_type': 'firefox',
6363
'location': 'San Francisco'
6464
}

‎tests/helpers_tests/test_condition.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ def setUp(self):
2626
self.config_dict['audiences'][0]['conditions']
2727
)
2828
attributes = {
29-
'test_attribute': 'test_value',
29+
'test_attribute': 'test_value_1',
3030
'browser_type': 'firefox',
3131
'location': 'San Francisco'
3232
}
@@ -121,4 +121,4 @@ def test_loads(self):
121121
)
122122

123123
self.assertEqual(['and', ['or', ['or', 0]]], condition_structure)
124-
self.assertEqual([['test_attribute', 'test_value']], condition_list)
124+
self.assertEqual([['test_attribute', 'test_value_1']], condition_list)

0 commit comments

Comments
 (0)