Skip to content

Commit

Permalink
Merge 496870b into 1ed413d
Browse files Browse the repository at this point in the history
  • Loading branch information
rashidsp committed Apr 24, 2019
2 parents 1ed413d + 496870b commit 692259a
Show file tree
Hide file tree
Showing 8 changed files with 273 additions and 164 deletions.
14 changes: 6 additions & 8 deletions optimizely/decision_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@
from .user_profile import UserProfile

Decision = namedtuple('Decision', 'experiment variation source')
DECISION_SOURCE_EXPERIMENT = 'EXPERIMENT'
DECISION_SOURCE_ROLLOUT = 'ROLLOUT'


class DecisionService(object):
Expand Down Expand Up @@ -215,7 +213,7 @@ def get_variation_for_rollout(self, rollout, user_id, attributes=None):
variation.key,
experiment.key
))
return Decision(experiment, variation, DECISION_SOURCE_ROLLOUT)
return Decision(experiment, variation, enums.DecisionSources.ROLLOUT)
else:
# Evaluate no further rules
self.logger.debug('User "%s" is not in the traffic group for the targeting else. '
Expand All @@ -233,9 +231,9 @@ def get_variation_for_rollout(self, rollout, user_id, attributes=None):
variation = self.bucketer.bucket(everyone_else_experiment, user_id, bucketing_id)
if variation:
self.logger.debug('User "%s" meets conditions for targeting rule "Everyone Else".' % user_id)
return Decision(everyone_else_experiment, variation, DECISION_SOURCE_ROLLOUT)
return Decision(everyone_else_experiment, variation, enums.DecisionSources.ROLLOUT)

return Decision(None, None, DECISION_SOURCE_ROLLOUT)
return Decision(None, None, enums.DecisionSources.ROLLOUT)

def get_experiment_in_group(self, group, bucketing_id):
""" Determine which experiment in the group the user is bucketed into.
Expand Down Expand Up @@ -296,7 +294,7 @@ def get_variation_for_feature(self, feature, user_id, attributes=None):
variation.key,
experiment.key
))
return Decision(experiment, variation, DECISION_SOURCE_EXPERIMENT)
return Decision(experiment, variation, enums.DecisionSources.FEATURE_TEST)
else:
self.logger.error(enums.Errors.INVALID_GROUP_ID_ERROR.format('_get_variation_for_feature'))

Expand All @@ -313,11 +311,11 @@ def get_variation_for_feature(self, feature, user_id, attributes=None):
variation.key,
experiment.key
))
return Decision(experiment, variation, DECISION_SOURCE_EXPERIMENT)
return Decision(experiment, variation, enums.DecisionSources.FEATURE_TEST)

# Next check if user is part of a rollout
if feature.rolloutId:
rollout = self.config.get_rollout_from_id(feature.rolloutId)
return self.get_variation_for_rollout(rollout, user_id, attributes)
else:
return Decision(None, None, DECISION_SOURCE_ROLLOUT)
return Decision(None, None, enums.DecisionSources.ROLLOUT)
14 changes: 10 additions & 4 deletions optimizely/helpers/enums.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,20 @@ class NotificationTypes(object):
TRACK notification listener has the following parameters:
str event_key, str user_id, dict attributes (can be None), event_tags (can be None), Event event
DECISION notification listener has the following parameters:
DecisionInfoTypes type, str user_id, dict attributes (can be None), dict decision_info
DecisionNotificationTypes type, str user_id, dict attributes, dict decision_info
"""
ACTIVATE = "ACTIVATE:experiment, user_id, attributes, variation, event"
DECISION = "DECISION:type, user_id, attributes, decision_info"
TRACK = "TRACK:event_key, user_id, attributes, event_tags, event"


class DecisionInfoTypes(object):
EXPERIMENT = "experiment"
class DecisionNotificationTypes(object):
AB_TEST = "ab-test"
FEATURE = "feature"
FEATURE_VARIABLE = "feature_variable"
FEATURE_TEST = 'feature-test'
FEATURE_VARIABLE = "feature-variable"


class DecisionSources(object):
FEATURE_TEST = 'feature-test'
ROLLOUT = 'rollout'
42 changes: 23 additions & 19 deletions optimizely/optimizely.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ def _get_feature_variable_for_type(self, feature_key, variable_key, variable_typ
return None

feature_enabled = False
source_info = {}
variable_value = variable.defaultValue
decision = self.decision_service.get_variation_for_feature(feature_flag, user_id, attributes)
if decision.variation:
Expand All @@ -232,12 +233,11 @@ def _get_feature_variable_for_type(self, feature_key, variable_key, variable_typ
'Returning default value for variable "%s" of feature flag "%s".' % (user_id, variable_key, feature_key)
)

experiment_key = None
variation_key = None

if decision.source == decision_service.DECISION_SOURCE_EXPERIMENT:
experiment_key = decision.experiment.key
variation_key = decision.variation.key
if decision.source == enums.DecisionSources.FEATURE_TEST:
source_info = {
'experiment_key': decision.experiment.key,
'variation_key': decision.variation.key
}

try:
actual_value = self.config.get_typecast_value(variable_value, variable_type)
Expand All @@ -247,18 +247,17 @@ def _get_feature_variable_for_type(self, feature_key, variable_key, variable_typ

self.notification_center.send_notifications(
enums.NotificationTypes.DECISION,
enums.DecisionInfoTypes.FEATURE_VARIABLE,
enums.DecisionNotificationTypes.FEATURE_VARIABLE,
user_id,
attributes or {},
{
'feature_key': feature_key,
'feature_enabled': feature_enabled,
'source': decision.source,
'variable_key': variable_key,
'variable_value': actual_value,
'variable_type': variable_type,
'source': decision.source,
'source_experiment_key': experiment_key,
'source_variation_key': variation_key
'source_info': source_info
}
)
return actual_value
Expand Down Expand Up @@ -388,9 +387,14 @@ def get_variation(self, experiment_key, user_id, attributes=None):
if variation:
variation_key = variation.key

if self.config.is_feature_experiment(experiment.id):
decision_notification_type = enums.DecisionNotificationTypes.FEATURE_TEST
else:
decision_notification_type = enums.DecisionNotificationTypes.AB_TEST

self.notification_center.send_notifications(
enums.NotificationTypes.DECISION,
enums.DecisionInfoTypes.EXPERIMENT,
decision_notification_type,
user_id,
attributes or {},
{
Expand Down Expand Up @@ -432,19 +436,20 @@ def is_feature_enabled(self, feature_key, user_id, attributes=None):
if not feature:
return False

experiment_key = None
feature_enabled = False
variation_key = None
source_info = {}
decision = self.decision_service.get_variation_for_feature(feature, user_id, attributes)
is_source_experiment = decision.source == decision_service.DECISION_SOURCE_EXPERIMENT
is_source_experiment = decision.source == enums.DecisionSources.FEATURE_TEST

if decision.variation:
if decision.variation.featureEnabled is True:
feature_enabled = True
# Send event if Decision came from an experiment.
if is_source_experiment:
experiment_key = decision.experiment.key
variation_key = decision.variation.key
source_info = {
'experiment_key': decision.experiment.key,
'variation_key': decision.variation.key
}
self._send_impression_event(decision.experiment,
decision.variation,
user_id,
Expand All @@ -457,15 +462,14 @@ def is_feature_enabled(self, feature_key, user_id, attributes=None):

self.notification_center.send_notifications(
enums.NotificationTypes.DECISION,
enums.DecisionInfoTypes.FEATURE,
enums.DecisionNotificationTypes.FEATURE,
user_id,
attributes or {},
{
'feature_key': feature_key,
'feature_enabled': feature_enabled,
'source': decision.source,
'source_experiment_key': experiment_key,
'source_variation_key': variation_key
'source_info': source_info
}
)

Expand Down
23 changes: 21 additions & 2 deletions optimizely/project_config.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright 2016-2018, Optimizely
# Copyright 2016-2019, Optimizely
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
Expand Down Expand Up @@ -106,12 +106,19 @@ def __init__(self, datafile, logger, error_handler):
)

self.feature_key_map = self._generate_key_map(self.feature_flags, 'key', entities.FeatureFlag)

# Hash containing experiment Ids that exist in any feature
# for checking that experiment is a feature experiment or not.
self.experiment_feature_map = {}
for feature in self.feature_key_map.values():
feature.variables = self._generate_key_map(feature.variables, 'key', entities.Variable)

# Check if any of the experiments are in a group and add the group id for faster bucketing later on
for exp_id in feature.experimentIds:
# Add this experiment in experiment-feature map.
self.experiment_feature_map[exp_id] = [feature.id]

experiment_in_feature = self.experiment_id_map[exp_id]
# Check if any of the experiments are in a group and add the group id for faster bucketing later on
if experiment_in_feature.groupId:
feature.groupId = experiment_in_feature.groupId
# Experiments in feature can only belong to one mutex group
Expand Down Expand Up @@ -609,3 +616,15 @@ def get_bot_filtering_value(self):
"""

return self.bot_filtering

def is_feature_experiment(self, experiment_id):
""" Determines if given experiment is a feature test.
Args:
experiment_id: Experiment ID for which feature test is to be determined.
Returns:
A boolean value that indicates if given experiment is a feature test.
"""

return experiment_id in self.experiment_feature_map
37 changes: 36 additions & 1 deletion tests/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,41 @@ def setUp(self, config_dict='config_dict'):
'id': '130', 'value': '4243'
}]
}]
}, {
'key': 'test_experiment2',
'status': 'Running',
'layerId': '5',
'audienceIds': [],
'id': '111133',
'forcedVariations': {},
'trafficAllocation': [{
'entityId': '122239',
'endOfRange': 5000
}, {
'entityId': '122240',
'endOfRange': 10000
}],
'variations': [{
'id': '122239',
'key': 'control',
'featureEnabled': True,
'variables': [
{
'id': '155551',
'value': '42.42'
}
]
}, {
'id': '122240',
'key': 'variation',
'featureEnabled': True,
'variables': [
{
'id': '155551',
'value': '13.37'
}
]
}]
}],
'groups': [{
'id': '19228',
Expand Down Expand Up @@ -431,7 +466,7 @@ def setUp(self, config_dict='config_dict'):
}, {
'id': '91114',
'key': 'test_feature_in_experiment_and_rollout',
'experimentIds': ['111127'],
'experimentIds': ['32223'],
'rolloutId': '211111',
'variables': [],
}]
Expand Down
18 changes: 18 additions & 0 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,11 @@ def test_init__with_v4_datafile(self):
}
}

expected_experiment_feature_map = {
'111127': ['91111'],
'32222': ['91113']
}

self.assertEqual(expected_variation_variable_usage_map['28901'],
project_config.variation_variable_usage_map['28901'])
self.assertEqual(expected_group_id_map, project_config.group_id_map)
Expand All @@ -639,6 +644,7 @@ def test_init__with_v4_datafile(self):
self.assertEqual(expected_feature_key_map, project_config.feature_key_map)
self.assertEqual(expected_rollout_id_map, project_config.rollout_id_map)
self.assertEqual(expected_variation_variable_usage_map, project_config.variation_variable_usage_map)
self.assertEqual(expected_experiment_feature_map, project_config.experiment_feature_map)

def test_variation_has_featureEnabled_false_if_prop_undefined(self):
""" Test that featureEnabled property by default is set to False, when not given in the data file"""
Expand Down Expand Up @@ -1333,3 +1339,15 @@ def test_get_group__invalid_id(self):
self.assertRaisesRegexp(exceptions.InvalidGroupException,
enums.Errors.INVALID_GROUP_ID_ERROR,
self.project_config.get_group, '42')

def test_is_feature_experiment(self):
""" Test that a true is returned if experiment is a feature test, false otherwise. """

opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
project_config = opt_obj.config

experiment = project_config.get_experiment_from_key('test_experiment2')
feature_experiment = project_config.get_experiment_from_key('test_experiment')

self.assertStrictFalse(project_config.is_feature_experiment(experiment.id))
self.assertStrictTrue(project_config.is_feature_experiment(feature_experiment.id))
Loading

0 comments on commit 692259a

Please sign in to comment.