fixed few more PR comments

optimizely · Mat001 · Dec 6, 2021 · Sep 9, 2021 · Sep 15, 2021 · Sep 16, 2021
commit e9cd3049b6857b864c51cbca247ecc8e9710e416
diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py
@@ -349,6 +349,9 @@ def get_variation_for_rollout(self, project_config, feature, user, options):
         if not feature:
             return Decision(None, None, enums.DecisionSources.ROLLOUT), decide_reasons
 
+        if not feature.rolloutId:
+            return Decision(None, None, enums.DecisionSources.ROLLOUT), decide_reasons
+
         rollout = project_config.get_rollout_from_id(feature.rolloutId)
 
         if not rollout:
@@ -375,7 +378,11 @@ def get_variation_for_rollout(self, project_config, feature, user, options):
 
                 decide_reasons += reasons_received
 
-                if decision_response:
+                if not decision_response:
+                    # TODO - MATJAZ - careful - check how this exists the loop and terminates properly
+                    #  when return is hit
+                    return Decision(None, None, enums.DecisionSources.ROLLOUT), decide_reasons
+                else:
                     variation, skip_to_everyone_else = decision_response
 
                 if variation:
@@ -431,6 +438,7 @@ def get_variation_from_delivery_rule(self, config, feature, rules, rule_index, u
           config: Instance of ProjectConfig.
           flag_key: Key of the flag.
           rules: Experiment rule.
+          rule_index: integer index of the rule in the list.
           user: ID and attributes for user.
           options: Decide options.
 
@@ -459,7 +467,8 @@ def get_variation_from_delivery_rule(self, config, feature, rules, rule_index, u
         # regular decision
         user_id = user.user_id
         attributes = user.get_user_attributes()
-        bucketing_id = self._get_bucketing_id(user_id, attributes)
+        # TODO this bucket_reasons go somewhere?
+        bucketing_id, bucket_reasons = self._get_bucketing_id(user_id, attributes)
 
         everyone_else = (rule_index == len(rules) - 1)
         logging_key = "Everyone Else" if everyone_else else str(rule_index + 1)
@@ -473,7 +482,6 @@ def get_variation_from_delivery_rule(self, config, feature, rules, rule_index, u
         decide_reasons += reasons_received_audience
 
         if audience_decision_response:
-
             message = 'User "{}" meets audience conditions for targeting rule {}.'.format(user_id, logging_key)
             self.logger.debug(message)
             decide_reasons.append(message)
@@ -535,10 +543,6 @@ def get_variation_for_feature(self, project_config, feature, user_context, ignor
         if feature.rolloutId:
             return self.get_variation_for_rollout(project_config, feature, user_context, ignore_user_profile)
 
-        # check if not part of experiment
-        if not feature.experimentIds:
-            return Decision(None, None, enums.DecisionSources.FEATURE_TEST), decide_reasons
-
         # check if not part of rollout
         if not feature.rolloutId:
             return Decision(None, None, enums.DecisionSources.ROLLOUT), decide_reasons
diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py
@@ -1048,8 +1048,8 @@ def _decide(self, user_context, key, decide_options=None):
         else:
             # Regular decision
             decision, decision_reasons = self.decision_service.get_variation_for_feature(config,
-                                                                       feature_flag,
-                                                                       user_context, ignore_ups)
+                                                                                         feature_flag,
+                                                                                         user_context, ignore_ups)
 
             reasons += decision_reasons
 

diff --git a/optimizely/project_config.py b/optimizely/project_config.py
@@ -214,7 +214,7 @@ def _deserialize_audience(audience_map):
         return audience_map
 
     def get_rollout_experiments(self, rollout):
-        """ Helper method to get rollout experiments as.
+        """ Helper method to get rollout experiments.
 
         Args:
             rollout: rollout

@@ -5074,4 +5074,3 @@ def test_invalid_flag_key(self):
         """
         # TODO mock function get_flag_variation_by_key?
         pass
-
Original file line number	Diff line number	Diff line change
Expand Up		@@ -5074,4 +5074,3 @@ def test_invalid_flag_key(self):
		"""
		# TODO mock function get_flag_variation_by_key?
		pass