Skip to content

Add forced-decisions APIs to OptimizelyUserContext #361

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 42 commits into from
Dec 6, 2021
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
c5d9dae
add maps to project config
Mat001 Sep 9, 2021
17ad742
initial code
Mat001 Sep 15, 2021
cee1fb8
Merge branch 'master' of github.com:optimizely/python-sdk into mpirno…
Mat001 Sep 16, 2021
58977d2
feat: add remaining implementation
Mat001 Oct 1, 2021
340cbce
WIP: addressed implementation PR comments and fixed failing unit tests
Mat001 Oct 19, 2021
c81a425
Fixed lint errors
Mat001 Oct 19, 2021
c89bc3c
fix failing tests in py 3.5
Mat001 Oct 20, 2021
2fe78ab
fixed failing logger import for Py2
Mat001 Oct 20, 2021
d80c555
add OptimizelyDecisionContext and OptmizelyForcedDecisions
Mat001 Oct 27, 2021
5ed2fb4
testcases added
ozayr-zaviar Nov 2, 2021
6003fdc
Update optimizely/optimizely_user_context.py
Mat001 Nov 2, 2021
e4dc745
Update optimizely/optimizely_user_context.py
Mat001 Nov 2, 2021
d75f389
Update optimizely/optimizely_user_context.py
Mat001 Nov 2, 2021
68146a1
make rule key optional in OptimizelyDecisionContext
Mat001 Nov 2, 2021
a261899
Mutex lock and testcases added
ozayr-zaviar Nov 3, 2021
a837f03
Merge branch 'master' into mpirnovar/forced_decisions
Mat001 Nov 3, 2021
de4a31c
Update optimizely/optimizely_user_context.py
Mat001 Nov 5, 2021
0c52707
use get() vs [] in remove_forced_decision
Mat001 Nov 5, 2021
4116b43
add self lock and keys(0
Mat001 Nov 5, 2021
081cd79
add missing colon
Mat001 Nov 7, 2021
e061abc
fix displaying reasons
Mat001 Nov 11, 2021
337f8d9
Update optimizely/optimizely.py
Mat001 Nov 12, 2021
a71f50e
address PR comments
Mat001 Nov 16, 2021
981cbe5
updating
Mat001 Nov 16, 2021
94d5af9
more PR review fixes
Mat001 Nov 17, 2021
e9cd304
fixed few more PR comments
Mat001 Nov 17, 2021
2dff4c6
added bucket reasons
Mat001 Nov 17, 2021
e2f1db3
FSC fixes
ozayr-zaviar Nov 18, 2021
6849c33
addressed more PR comments, fixed FSC test failuer about impressin ev…
Mat001 Nov 19, 2021
55fe98f
address more PR comments
Mat001 Nov 19, 2021
94a0c26
use is_valid check on opti client
Mat001 Nov 19, 2021
e5aaccb
addressed more PR comments
Mat001 Nov 19, 2021
44373e9
reasons and key name fixed
ozayr-zaviar Nov 22, 2021
e6c1772
create get_default method for empty experiment object
Mat001 Nov 22, 2021
ab40d9e
fixed further PR comments
Mat001 Nov 24, 2021
795b41a
fix logger so we use the top logger in optimizely client
Mat001 Dec 1, 2021
dbbc051
Refact: Refactored Forced decision (#365)
msohailhussain Dec 2, 2021
75fe2bb
coupl of corrections
Mat001 Dec 2, 2021
243d447
remove check on config
Mat001 Dec 2, 2021
1010ece
remove redundant import
Mat001 Dec 2, 2021
17efc27
remove redundant test about invalid datafile
Mat001 Dec 3, 2021
201548f
add reasons to return
Mat001 Dec 4, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
230 changes: 104 additions & 126 deletions optimizely/decision_service.py

Large diffs are not rendered by default.

17 changes: 17 additions & 0 deletions optimizely/entities.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,23 @@ def get_audience_conditions_or_ids(self):
def __str__(self):
return self.key

@staticmethod
def get_default():
""" returns an empty experiment object. """
experiment = Experiment(
id='',
key='',
layerId='',
status='',
variations=[],
trafficAllocation=[],
audienceIds=[],
audienceConditions=[],
forcedVariations={}
)

return experiment


class FeatureFlag(BaseEntity):
def __init__(self, id, key, experimentIds, rolloutId, variables, groupId=None, **kwargs):
Expand Down
3 changes: 3 additions & 0 deletions optimizely/event/user_event_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ def create_impression_event(

if variation_id and experiment_id:
variation = project_config.get_variation_from_id_by_experiment_id(experiment_id, variation_id)
# need this condition when we send events involving forced decisions
elif variation_id and flag_key:
variation = project_config.get_flag_variation(flag_key, 'id', variation_id)
event_context = user_event.EventContext(
project_config.account_id, project_config.project_id, project_config.revision, project_config.anonymize_ip,
)
Expand Down
44 changes: 7 additions & 37 deletions optimizely/optimizely.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,9 @@ def _send_impression_event(self, project_config, experiment, variation, flag_key
user_id: ID for user.
attributes: Dict representing user attributes and values which need to be recorded.
"""
if not experiment:
experiment = entities.Experiment.get_default()

variation_id = variation.id if variation is not None else None
user_event = user_event_factory.UserEventFactory.create_impression_event(
project_config, experiment, variation_id, flag_key, rule_key, rule_type, enabled, user_id, attributes
Expand Down Expand Up @@ -293,9 +296,6 @@ def _get_feature_variable_for_type(
)

if decision.source == enums.DecisionSources.FEATURE_TEST:

print('DECISION.EXPERIMENT - ', decision.experiment)

source_info = {
'experiment_key': decision.experiment.key,
'variation_key': decision.variation.key,
Expand Down Expand Up @@ -1033,13 +1033,10 @@ def _decide(self, user_context, key, decide_options=None):
decision_source = DecisionSources.ROLLOUT
source_info = {}
decision_event_dispatched = False
ignore_ups = OptimizelyDecideOption.IGNORE_USER_PROFILE_SERVICE in decide_options

# Check forced decisions first
optimizely_decision_context = OptimizelyUserContext.OptimizelyDecisionContext(flag_key=key, rule_key=rule_key)
forced_decision_response = user_context.find_validated_forced_decision(optimizely_decision_context,
options=decide_options)

forced_decision_response = user_context.find_validated_forced_decision(optimizely_decision_context)
variation, decision_reasons = forced_decision_response
reasons += decision_reasons

Expand All @@ -1049,15 +1046,15 @@ def _decide(self, user_context, key, decide_options=None):
# Regular decision
decision, decision_reasons = self.decision_service.get_variation_for_feature(config,
feature_flag,
user_context, ignore_ups)
user_context, decide_options)

reasons += decision_reasons

# Fill in experiment and variation if returned (rollouts can have featureEnabled variables as well.)
if decision.experiment is not None:
experiment = decision.experiment
source_info["experiment"] = experiment
rule_key = experiment.key
rule_key = experiment.key if experiment else None
if decision.variation is not None:
variation = decision.variation
variation_key = variation.key
Expand All @@ -1072,6 +1069,7 @@ def _decide(self, user_context, key, decide_options=None):
self._send_impression_event(config, experiment, variation, flag_key, rule_key or '',
decision_source, feature_enabled,
user_id, attributes)

decision_event_dispatched = True

# Generate all variables map if decide options doesn't include excludeVariables
Expand Down Expand Up @@ -1185,31 +1183,3 @@ def _decide_for_keys(self, user_context, keys, decide_options=None):
continue
decisions[key] = decision
return decisions

def get_flag_variation_by_key(self, flag_key, variation_key):
"""
Gets variation by key.
variation_key can be a string or in case of forced decisions, it can be an object.

Args:
flag_key: flag key
variation_key: variation key

Returns:
Variation as a map.
"""
config = self.config_manager.get_config()

if not config:
return None

if not flag_key:
return None

variations = config.flag_variations_map.get(flag_key)

for variation in variations:
if variation.key == variation_key:
return variation

return None
63 changes: 43 additions & 20 deletions optimizely/optimizely_user_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,15 @@ def __init__(self, optimizely_client, user_id, user_attributes=None):

self._user_attributes = user_attributes.copy() if user_attributes else {}
self.lock = threading.Lock()
self.forced_decisions = {}
self.forced_decisions_map = {}
self.log = logger.SimpleLogger(min_level=enums.LogLevels.INFO)

# decision context
class OptimizelyDecisionContext(object):
""" Using class with attributes here instead of namedtuple because
class is extensible, it's easy to add another attribute if we wanted
to extend decision context.
"""
def __init__(self, flag_key, rule_key=None):
self.flag_key = flag_key
self.rule_key = rule_key
Expand All @@ -73,8 +77,8 @@ def _clone(self):
user_context = OptimizelyUserContext(self.client, self.user_id, self.get_user_attributes())

with self.lock:
if self.forced_decisions:
user_context.forced_decisions = copy.deepcopy(self.forced_decisions)
if self.forced_decisions_map:
user_context.forced_decisions_map = copy.deepcopy(self.forced_decisions_map)

return user_context

Expand Down Expand Up @@ -159,12 +163,12 @@ def set_forced_decision(self, decision_context, decision):
Returns:
True if the forced decision has been set successfully.
"""
if not self.client.config_manager.get_config():
if not self.client.is_valid or not self.client.config_manager.get_config():
self.log.logger.error(OptimizelyDecisionMessage.SDK_NOT_READY)
return False

with self.lock:
self.forced_decisions[decision_context] = decision
self.forced_decisions_map[decision_context] = decision

return True

Expand All @@ -178,13 +182,13 @@ def get_forced_decision(self, decision_context):
Returns:
A forced_decision or None if forced decisions are not set for the parameters.
"""
if not self.client.config_manager.get_config():
if not self.client.is_valid or not self.client.config_manager.get_config():
self.log.logger.error(OptimizelyDecisionMessage.SDK_NOT_READY)
return None

forced_decision = self.find_forced_decision(decision_context)

return forced_decision if forced_decision else None
return forced_decision

def remove_forced_decision(self, decision_context):
"""
Expand All @@ -196,13 +200,13 @@ def remove_forced_decision(self, decision_context):
Returns:
Returns: true if the forced decision has been removed successfully.
"""
if not self.client.config_manager.get_config():
if not self.client.is_valid or not self.client.config_manager.get_config():
self.log.logger.error(OptimizelyDecisionMessage.SDK_NOT_READY)
return False

with self.lock:
if decision_context in self.forced_decisions:
del self.forced_decisions[decision_context]
if decision_context in self.forced_decisions_map:
del self.forced_decisions_map[decision_context]
return True

return False
Expand All @@ -214,46 +218,65 @@ def remove_all_forced_decisions(self):
Returns:
True if forced decisions have been removed successfully.
"""
if not self.client.config_manager.get_config():
if not self.client.is_valid or not self.client.config_manager.get_config():
self.log.logger.error(OptimizelyDecisionMessage.SDK_NOT_READY)
return False

with self.lock:
self.forced_decisions.clear()
self.forced_decisions_map.clear()

return True

def find_forced_decision(self, decision_context):
"""
Gets forced decision from forced decision map.

Args:
decision_context: a decision context.

Returns:
Forced decision.
"""
with self.lock:
if not self.forced_decisions:
if not self.forced_decisions_map:
return None

# must allow None to be returned for the Flags only case
return self.forced_decisions.get(decision_context)
return self.forced_decisions_map.get(decision_context)

def find_validated_forced_decision(self, decision_context, options):
def find_validated_forced_decision(self, decision_context):
"""
Gets forced decisions based on flag key, rule key and variation.

Args:
decision context: a decision context

Returns:
Variation of the forced decision.
"""
reasons = []

forced_decision_response = self.find_forced_decision(decision_context)
forced_decision = self.find_forced_decision(decision_context)

flag_key = decision_context.flag_key
rule_key = decision_context.rule_key

if forced_decision_response:
variation = self.client.get_flag_variation_by_key(flag_key, forced_decision_response.variation_key)
if forced_decision:
# we use config here so we can use get_flag_variation() function which is defined in project_config
# otherwise we would us self.client instead of config
config = self.client.config_manager.get_config() if self.client else None
variation = config.get_flag_variation(flag_key, 'key', forced_decision.variation_key)
if variation:
if rule_key:
user_has_forced_decision = enums.ForcedDecisionLogs \
.USER_HAS_FORCED_DECISION_WITH_RULE_SPECIFIED.format(forced_decision_response.variation_key,
.USER_HAS_FORCED_DECISION_WITH_RULE_SPECIFIED.format(forced_decision.variation_key,
flag_key,
rule_key,
self.user_id)

else:
user_has_forced_decision = enums.ForcedDecisionLogs \
.USER_HAS_FORCED_DECISION_WITHOUT_RULE_SPECIFIED.format(forced_decision_response.variation_key,
.USER_HAS_FORCED_DECISION_WITHOUT_RULE_SPECIFIED.format(forced_decision.variation_key,
flag_key,
self.user_id)

Expand Down
Loading