Skip to content

Commit 94d5af9

Browse files
Mat001Mat001
committed
more PR review fixes
1 parent 981cbe5 commit 94d5af9

File tree

4 files changed

+33
-22
lines changed

4 files changed

+33
-22
lines changed

‎optimizely/optimizely.py

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,9 @@ def _get_feature_variable_for_type(
293293
)
294294

295295
if decision.source == enums.DecisionSources.FEATURE_TEST:
296+
297+
print('DECISION.EXPERIMENT - ', decision.experiment)
298+
296299
source_info = {
297300
'experiment_key': decision.experiment.key,
298301
'variation_key': decision.variation.key,
@@ -444,20 +447,14 @@ def activate(self, experiment_key, user_id, attributes=None):
444447
self.logger.error(enums.Errors.INVALID_PROJECT_CONFIG.format('activate'))
445448
return None
446449

447-
variation = self.get_variation(experiment_key, user_id, attributes)
448-
449-
if not variation:
450-
self.logger.info('Not activating user "%s".' % user_id)
451-
return None
452-
453-
variation_key, reasons = variation
450+
variation_key = self.get_variation(experiment_key, user_id, attributes)
454451

455452
if not variation_key:
456453
self.logger.info('Not activating user "%s".' % user_id)
457454
return None
458455

459456
experiment = project_config.get_experiment_from_key(experiment_key)
460-
variation = project_config.get_variation_from_key(experiment_key, variation_key.key)
457+
variation = project_config.get_variation_from_key(experiment_key, variation_key)
461458

462459
# Create and dispatch impression event
463460
self.logger.info('Activating user "%s" in experiment "%s".' % (user_id, experiment.key))
@@ -555,7 +552,10 @@ def get_variation(self, experiment_key, user_id, attributes=None):
555552
return None
556553

557554
user_context = self.create_user_context(user_id, attributes)
558-
variation_key = self.decision_service.get_variation(project_config, experiment, user_context)
555+
556+
variation, _ = self.decision_service.get_variation(project_config, experiment, user_context)
557+
if variation:
558+
variation_key = variation.key
559559

560560
if project_config.is_feature_experiment(experiment.id):
561561
decision_notification_type = enums.DecisionNotificationTypes.FEATURE_TEST
@@ -841,7 +841,7 @@ def get_feature_variable_json(self, feature_key, variable_key, user_id, attribut
841841
project_config, feature_key, variable_key, variable_type, user_id, attributes,
842842
)
843843

844-
def get_all_feature_variables(self, feature_key, user_id, attributes):
844+
def get_all_feature_variables(self, feature_key, user_id, attributes=None):
845845
""" Returns dictionary of all variables and their corresponding values in the context of a feature.
846846
847847
Args:
@@ -1047,10 +1047,10 @@ def _decide(self, user_context, key, decide_options=None):
10471047
decision = Decision(None, variation, enums.DecisionSources.FEATURE_TEST)
10481048
else:
10491049
# Regular decision
1050-
decision = decision_service.DecisionService.get_variation_for_feature(self.decision_service, config,
1051-
feature_flag,
1052-
user_context, ignore_ups)
1053-
decision, decision_reasons = decision
1050+
decision, decision_reasons = self.decision_service.get_variation_for_feature(config,
1051+
feature_flag,
1052+
user_context, ignore_ups)
1053+
10541054
reasons += decision_reasons
10551055

10561056
# Fill in experiment and variation if returned (rollouts can have featureEnabled variables as well.)
@@ -1203,7 +1203,10 @@ def get_flag_variation_by_key(self, flag_key, variation_key):
12031203
if not config:
12041204
return None
12051205

1206-
variations = config.flag_variations_map[flag_key]
1206+
if not flag_key:
1207+
return None
1208+
1209+
variations = config.flag_variations_map.get(flag_key)
12071210

12081211
for variation in variations:
12091212
if variation.key == variation_key:

‎optimizely/optimizely_user_context.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -176,19 +176,19 @@ def get_forced_decision(self, decision_context):
176176
decision_context: a decision context.
177177
178178
Returns:
179-
A variation key or None if forced decisions are not set for the parameters.
179+
A forced_decision or None if forced decisions are not set for the parameters.
180180
"""
181181
if not self.client.config_manager.get_config():
182182
self.log.logger.error(OptimizelyDecisionMessage.SDK_NOT_READY)
183183
return None
184184

185-
forced_decision_key = self.find_forced_decision(decision_context)
185+
forced_decision = self.find_forced_decision(decision_context)
186186

187-
return forced_decision_key if forced_decision_key else None
187+
return forced_decision if forced_decision else None
188188

189189
def remove_forced_decision(self, decision_context):
190190
"""
191-
Removes the forced decision for a given flag and an optional rule.
191+
Removes the forced decision for a given decision context.
192192
193193
Args:
194194
decision_context: a decision context.
@@ -201,7 +201,7 @@ def remove_forced_decision(self, decision_context):
201201
return False
202202

203203
with self.lock:
204-
if decision_context in self.forced_decisions.keys():
204+
if decision_context in self.forced_decisions:
205205
del self.forced_decisions[decision_context]
206206
return True
207207

‎optimizely/project_config.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ def __init__(self, datafile, logger, error_handler):
155155
if rollout and rollout.experiments:
156156
experiments.extend(rollout_experiments)
157157

158-
self.flag_rules_map[flag['key']] = experiments
158+
self.flag_rules_map[flag['key']] = experiments
159159

160160
# All variations for each flag
161161
# Datafile does not contain a separate entity for this.
@@ -214,7 +214,7 @@ def _deserialize_audience(audience_map):
214214
return audience_map
215215

216216
def get_rollout_experiments(self, rollout):
217-
""" Helper method to get rollout experiments as a map.
217+
""" Helper method to get rollout experiments as.
218218
219219
Args:
220220
rollout: rollout

‎tests/test_optimizely.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5067,3 +5067,11 @@ def test_user_context_invalid_user_id(self):
50675067
for u in user_ids:
50685068
uc = self.optimizely.create_user_context(u)
50695069
self.assertIsNone(uc, "invalid user id should return none")
5070+
5071+
def test_invalid_flag_key(self):
5072+
"""
5073+
Tests invalid flag key in function get_flag_variation_by_key().
5074+
"""
5075+
# TODO mock function get_flag_variation_by_key?
5076+
pass
5077+

0 commit comments

Comments
 (0)