Skip to content

Commit fcc840f

Browse files
aliabbasrizvialiabbasrizvi
committed
Revert "Updating evaluation of rollout rules (#72)"
This reverts commit 669bca2.
1 parent aa03914 commit fcc840f

File tree

7 files changed

+126
-272
lines changed

7 files changed

+126
-272
lines changed

‎optimizely/decision_service.py

Lines changed: 10 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -175,59 +175,29 @@ def get_variation(self, experiment, user_id, attributes, ignore_user_profile=Fal
175175

176176
return None
177177

178-
def get_variation_for_rollout(self, rollout, user_id, attributes=None, ignore_user_profile=False):
179-
""" Determine which variation the user is in for a given rollout.
178+
def get_variation_for_layer(self, layer, user_id, attributes=None, ignore_user_profile=False):
179+
""" Determine which variation the user is in for a given layer.
180180
Returns the variation of the first experiment the user qualifies for.
181181
182182
Args:
183-
rollout: Rollout for which we are getting the variation.
183+
layer: Layer for which we are getting the variation.
184184
user_id: ID for user.
185185
attributes: Dict representing user attributes.
186186
ignore_user_profile: True to ignore the user profile lookup. Defaults to False.
187187
188+
188189
Returns:
189190
Variation the user should see. None if the user is not in any of the layer's experiments.
190191
"""
191-
192192
# Go through each experiment in order and try to get the variation for the user
193-
if rollout and len(rollout.experiments) > 0:
194-
for idx in range(len(rollout.experiments) - 1):
195-
experiment = self.config.get_experiment_from_key(rollout.experiments[idx].get('key'))
196-
197-
# Check if user meets audience conditions for targeting rule
198-
if not audience_helper.is_user_in_experiment(self.config, experiment, attributes):
199-
self.logger.log(
200-
enums.LogLevels.DEBUG,
201-
'User "%s" does not meet conditions for targeting rule %s.' % (user_id, idx + 1)
202-
)
203-
continue
204-
205-
self.logger.log(enums.LogLevels.DEBUG, 'User "%s" meets conditions for targeting rule %s.' % (user_id, idx + 1))
206-
# Determine bucketing ID to be used
207-
bucketing_id = self._get_bucketing_id(user_id, attributes)
208-
variation = self.bucketer.bucket(experiment, user_id, bucketing_id)
193+
if layer:
194+
for experiment_dict in layer.experiments:
195+
experiment = self.config.get_experiment_from_key(experiment_dict['key'])
196+
variation = self.get_variation(experiment, user_id, attributes, ignore_user_profile)
209197
if variation:
210198
self.logger.log(enums.LogLevels.DEBUG,
211199
'User "%s" is in variation %s of experiment %s.' % (user_id, variation.key, experiment.key))
212-
return variation
213-
else:
214-
# Evaluate no further rules
215-
self.logger.log(enums.LogLevels.DEBUG,
216-
'User "%s" is not in the traffic group for the targeting else. '
217-
'Checking "Everyone Else" rule now.' % user_id)
218-
break
219-
220-
# Evaluate last rule i.e. "Everyone Else" rule
221-
everyone_else_experiment = rollout.experiments[-1]
222-
if audience_helper.is_user_in_experiment(self.config,
223-
self.config.get_experiment_from_key(rollout.experiments[-1].get('key')),
224-
attributes):
225-
# Determine bucketing ID to be used
226-
bucketing_id = self._get_bucketing_id(user_id, attributes)
227-
variation = self.bucketer.bucket(everyone_else_experiment, user_id, bucketing_id)
228-
if variation:
229-
self.logger.log(enums.LogLevels.DEBUG,
230-
'User "%s" meets conditions for targeting rule "Everyone Else".' % user_id)
200+
# Return as soon as we get a variation
231201
return variation
232202

233203
return None
@@ -269,7 +239,6 @@ def get_variation_for_feature(self, feature, user_id, attributes=None):
269239
Returns:
270240
Variation that the user is bucketed in. None if the user is not in any variation.
271241
"""
272-
273242
variation = None
274243
bucketing_id = self._get_bucketing_id(user_id, attributes)
275244

@@ -301,6 +270,6 @@ def get_variation_for_feature(self, feature, user_id, attributes=None):
301270
# Next check if user is part of a rollout
302271
if not variation and feature.rolloutId:
303272
rollout = self.config.get_layer_from_id(feature.rolloutId)
304-
variation = self.get_variation_for_rollout(rollout, user_id, attributes, ignore_user_profile=True)
273+
variation = self.get_variation_for_layer(rollout, user_id, attributes, ignore_user_profile=True)
305274

306275
return variation

‎tests/base.py

Lines changed: 21 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -131,15 +131,10 @@ def setUp(self):
131131
'id': '111094'
132132
}],
133133
'audiences': [{
134-
'name': 'Test attribute users 1',
134+
'name': 'Test attribute users',
135135
'conditions': '["and", ["or", ["or", '
136-
'{"name": "test_attribute", "type": "custom_attribute", "value": "test_value_1"}]]]',
136+
'{"name": "test_attribute", "type": "custom_attribute", "value": "test_value"}]]]',
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'
143138
}],
144139
'projectId': '111001'
145140
}
@@ -247,15 +242,10 @@ def setUp(self):
247242
'id': '111094'
248243
}],
249244
'audiences': [{
250-
'name': 'Test attribute users 1',
245+
'name': 'Test attribute users',
251246
'conditions': '["and", ["or", ["or", '
252-
'{"name": "test_attribute", "type": "custom_attribute", "value": "test_value_1"}]]]',
247+
'{"name": "test_attribute", "type": "custom_attribute", "value": "test_value"}]]]',
253248
'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'
259249
}],
260250
'rollouts': [{
261251
'id': '211111',
@@ -267,48 +257,24 @@ def setUp(self):
267257
'layerId': '211111',
268258
'audienceIds': ['11154'],
269259
'trafficAllocation': [{
260+
'entityId': '211128',
261+
'endOfRange': 5000
262+
}, {
270263
'entityId': '211129',
271264
'endOfRange': 9000
272265
}],
273266
'variations': [{
267+
'key': '211128',
268+
'id': '211128'
269+
}, {
274270
'key': '211129',
275271
'id': '211129'
276272
}]
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-
}]
307273
}]
308274
}],
309275
'featureFlags': [{
310276
'id': '91111',
311-
'key': 'test_feature_in_experiment',
277+
'key': 'test_feature_1',
312278
'experimentIds': ['111127'],
313279
'rolloutId': '',
314280
'variables': [{
@@ -324,7 +290,7 @@ def setUp(self):
324290
}]
325291
}, {
326292
'id': '91112',
327-
'key': 'test_feature_in_rollout',
293+
'key': 'test_feature_2',
328294
'experimentIds': [],
329295
'rolloutId': '211111',
330296
'variables': [],
@@ -458,13 +424,8 @@ def setUp(self):
458424
'audiences': [{
459425
'name': 'Test attribute users',
460426
'conditions': '["and", ["or", ["or", '
461-
'{"name": "test_attribute", "type": "custom_attribute", "value": "test_value_1"}]]]',
427+
'{"name": "test_attribute", "type": "custom_attribute", "value": "test_value"}]]]',
462428
'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'
468429
}],
469430
'projectId': '111001'
470431
}
@@ -513,15 +474,10 @@ def setUp(self):
513474
'id': '111094'
514475
}],
515476
'audiences': [{
516-
'name': 'Test attribute users 1',
477+
'name': 'Test attribute users',
517478
'conditions': '["and", ["or", ["or", '
518-
'{"name": "test_attribute", "type": "custom_attribute", "value": "test_value_1"}]]]',
479+
'{"name": "test_attribute", "type": "custom_attribute", "value": "test_value"}]]]',
519480
'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'
525481
}],
526482
'rollouts': [{
527483
'id': '211111',
@@ -533,43 +489,19 @@ def setUp(self):
533489
'layerId': '211111',
534490
'audienceIds': ['11154'],
535491
'trafficAllocation': [{
492+
'entityId': '211128',
493+
'endOfRange': 5000
494+
}, {
536495
'entityId': '211129',
537496
'endOfRange': 9000
538497
}],
539498
'variations': [{
499+
'key': '211128',
500+
'id': '211128'
501+
}, {
540502
'key': '211129',
541503
'id': '211129'
542504
}]
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-
}]
573505
}]
574506
}],
575507
'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_1',
26+
'test_attribute': 'test_value',
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_1',
48+
'test_attribute': 'test_value',
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_1',
61+
'test_attribute': 'test_value',
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_1',
29+
'test_attribute': 'test_value',
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_1']], condition_list)
124+
self.assertEqual([['test_attribute', 'test_value']], condition_list)

0 commit comments

Comments
 (0)