Merge pull request #162 from splitio/development

Development

Author: mredolatti

.travis.yml

@@ -1,12 +1,21 @@
 language: python
 
-python:
-  - "2.7"
-  - "3.6"
+git:
+  depth: false
+
+matrix:
+  include:
+    - python: '2.7'
+    - python: '3.6'
+      after_success:
+      - bash sonar-scanner.sh
 
 services:
   - redis-server
 
+addons:
+  sonarqube: true
+
 install:
   - pip install -U setuptools pip
   - pip install -e .[cpphash,redis,uwsgi]

CHANGES.txt

@@ -1,3 +1,6 @@
+8.1.6 (Oct 31, 2019)
+ - Fixed input validation performance issue.
+
 8.1.5 (Oct 15, 2019)
  - Added logic to fetch multiple splits at once on get_treatments/get_treatments_with_config.
  - Added flag `IPAddressesEnabled` into config to enable/disable sending machineName and machineIp when data is posted in headers.

sonar-scanner.sh

@@ -0,0 +1,41 @@
+#/bin/bash -e
+
+sonar_scanner() {
+  local params=$@
+
+  sonar-scanner \
+    -Dsonar.host.url='https://sonarqube.split-internal.com' \
+    -Dsonar.login="$SONAR_TOKEN" \
+    -Dsonar.ws.timeout='300' \
+    -Dsonar.sources='splitio' \
+    -Dsonar.projectName='python-client' \
+    -Dsonar.projectKey='python-client' \
+    -Dsonar.links.ci='https://travis-ci.com/splitio/python-client' \
+    -Dsonar.links.scm='https://github.com/splitio/python-client' \
+    ${params}
+
+  return $?
+}
+
+if [ "$TRAVIS_PULL_REQUEST" != "false" ]; then
+  sonar_scanner \
+    -Dsonar.pullrequest.provider='GitHub' \
+    -Dsonar.pullrequest.github.repository='splitio/python-client' \
+    -Dsonar.pullrequest.key=$TRAVIS_PULL_REQUEST \
+    -Dsonar.pullrequest.branch=$TRAVIS_PULL_REQUEST_BRANCH \
+    -Dsonar.pullrequest.base=$TRAVIS_BRANCH
+else
+  if [ "$TRAVIS_BRANCH" == 'master' ]; then
+    sonar_scanner \
+      -Dsonar.branch.name=$TRAVIS_BRANCH
+  else
+    if [ "$TRAVIS_BRANCH" == 'development' ]; then
+      TARGET_BRANCH='master'
+    else
+      TARGET_BRANCH='development'
+    fi
+    sonar_scanner \
+      -Dsonar.branch.name=$TRAVIS_BRANCH \
+      -Dsonar.branch.target=$TARGET_BRANCH
+  fi
+fi

splitio/client/client.py

@@ -19,6 +19,8 @@ class Client(object):  # pylint: disable=too-many-instance-attributes
 
     _METRIC_GET_TREATMENT = 'sdk.getTreatment'
     _METRIC_GET_TREATMENTS = 'sdk.getTreatments'
+    _METRIC_GET_TREATMENT_WITH_CONFIG = 'sdk.getTreatmentWithConfig'
+    _METRIC_GET_TREATMENTS_WITH_CONFIG = 'sdk.getTreatmentsWithConfig'
 
     def __init__(self, factory, labels_enabled=True, impression_listener=None):
         """
@@ -103,39 +105,25 @@ def _evaluate_if_ready(self, matching_key, bucketing_key, feature, attributes=No
             attributes
         )
 
-    def get_treatment_with_config(self, key, feature, attributes=None):
-        """
-        Get the treatment and config for a feature and key, with optional dictionary of attributes.
-
-        This method never raises an exception. If there's a problem, the appropriate log message
-        will be generated and the method will return the CONTROL treatment.
-
-        :param key: The key for which to get the treatment
-        :type key: str
-        :param feature: The name of the feature for which to get the treatment
-        :type feature: str
-        :param attributes: An optional dictionary of attributes
-        :type attributes: dict
-        :return: The treatment for the key and feature
-        :rtype: tuple(str, str)
-        """
+    def _make_evaluation(self, key, feature, attributes, method_name, metric_name):
         try:
             if self.destroyed:
                 self._logger.error("Client has already been destroyed - no calls possible")
                 return CONTROL, None
 
             start = int(round(time.time() * 1000))
 
-            matching_key, bucketing_key = input_validator.validate_key(key)
+            matching_key, bucketing_key = input_validator.validate_key(key, method_name)
             feature = input_validator.validate_feature_name(
                 feature,
                 self.ready,
-                self._factory._get_storage('splits')  # pylint: disable=protected-access
+                self._factory._get_storage('splits'),  # pylint: disable=protected-access
+                method_name
             )
 
             if (matching_key is None and bucketing_key is None) \
                     or feature is None \
-                    or not input_validator.validate_attributes(attributes):
+                    or not input_validator.validate_attributes(attributes, method_name):
                 return CONTROL, None
 
             result = self._evaluate_if_ready(matching_key, bucketing_key, feature, attributes)
@@ -150,7 +138,7 @@ def get_treatment_with_config(self, key, feature, attributes=None):
                 start
             )
 
-            self._record_stats(impression, start, self._METRIC_GET_TREATMENT)
+            self._record_stats([impression], start, metric_name)
             self._send_impression_to_listener(impression, attributes)
             return result['treatment'], result['configurations']
         except Exception:  # pylint: disable=broad-except
@@ -166,80 +154,29 @@ def get_treatment_with_config(self, key, feature, attributes=None):
                     bucketing_key,
                     start
                 )
-                self._record_stats(impression, start, self._METRIC_GET_TREATMENT)
+                self._record_stats([impression], start, metric_name)
                 self._send_impression_to_listener(impression, attributes)
             except Exception:  # pylint: disable=broad-except
                 self._logger.error('Error reporting impression into get_treatment exception block')
                 self._logger.debug('Error: ', exc_info=True)
             return CONTROL, None
 
-    def get_treatment(self, key, feature, attributes=None):
-        """
-        Get the treatment for a feature and key, with an optional dictionary of attributes.
-
-        This method never raises an exception. If there's a problem, the appropriate log message
-        will be generated and the method will return the CONTROL treatment.
-
-        :param key: The key for which to get the treatment
-        :type key: str
-        :param feature: The name of the feature for which to get the treatment
-        :type feature: str
-        :param attributes: An optional dictionary of attributes
-        :type attributes: dict
-        :return: The treatment for the key and feature
-        :rtype: str
-        """
-        treatment, _ = self.get_treatment_with_config(key, feature, attributes)
-        return treatment
-
-    def _evaluate_features_if_ready(self, matching_key, bucketing_key, features, attributes=None):
-        if not self.ready:
-            return {
-                feature: {
-                    'treatment': CONTROL,
-                    'configurations': None,
-                    'impression': {'label': Label.NOT_READY, 'change_number': None}
-                }
-                for feature in features
-            }
-
-        return self._evaluator.evaluate_features(
-            features,
-            matching_key,
-            bucketing_key,
-            attributes
-        )
-
-    def get_treatments_with_config(self, key, features, attributes=None):
-        """
-        Evaluate multiple features and return a dict with feature -> (treatment, config).
-
-        Get the treatments for a list of features considering a key, with an optional dictionary of
-        attributes. This method never raises an exception. If there's a problem, the appropriate
-        log message will be generated and the method will return the CONTROL treatment.
-        :param key: The key for which to get the treatment
-        :type key: str
-        :param features: Array of the names of the features for which to get the treatment
-        :type feature: list
-        :param attributes: An optional dictionary of attributes
-        :type attributes: dict
-        :return: Dictionary with the result of all the features provided
-        :rtype: dict
-        """
+    def _make_evaluations(self, key, features, attributes, method_name, metric_name):
         if self.destroyed:
             self._logger.error("Client has already been destroyed - no calls possible")
-            return input_validator.generate_control_treatments(features)
+            return input_validator.generate_control_treatments(features, method_name)
 
         start = int(round(time.time() * 1000))
 
-        matching_key, bucketing_key = input_validator.validate_key(key)
+        matching_key, bucketing_key = input_validator.validate_key(key, method_name)
         if matching_key is None and bucketing_key is None:
-            return input_validator.generate_control_treatments(features)
+            return input_validator.generate_control_treatments(features, method_name)
 
-        if input_validator.validate_attributes(attributes) is False:
-            return input_validator.generate_control_treatments(features)
+        if input_validator.validate_attributes(attributes, method_name) is False:
+            return input_validator.generate_control_treatments(features, method_name)
 
         features, missing = input_validator.validate_features_get_treatments(
+            method_name,
             features,
             self.ready,
             self._factory._get_storage('splits')  # pylint: disable=protected-access
@@ -269,8 +206,8 @@ def get_treatments_with_config(self, key, features, attributes=None):
                     treatments[feature] = (result['treatment'], result['configurations'])
 
                 except Exception:  # pylint: disable=broad-except
-                    self._logger.error('get_treatments: An exception occured when evaluating '
-                                       'feature ' + feature + ' returning CONTROL.')
+                    self._logger.error('%s: An exception occured when evaluating '
+                                       'feature %s returning CONTROL.' % (method_name, feature))
                     treatments[feature] = CONTROL, None
                     self._logger.debug('Error: ', exc_info=True)
                     continue
@@ -282,15 +219,91 @@ def get_treatments_with_config(self, key, features, attributes=None):
                     for impression in bulk_impressions:
                         self._send_impression_to_listener(impression, attributes)
             except Exception:  # pylint: disable=broad-except
-                self._logger.error('get_treatments: An exception when trying to store '
-                                   'impressions.')
+                self._logger.error('%s: An exception when trying to store '
+                                   'impressions.' % method_name)
                 self._logger.debug('Error: ', exc_info=True)
 
             return treatments
         except Exception:  # pylint: disable=broad-except
             self._logger.error('Error getting treatment for features')
             self._logger.debug('Error: ', exc_info=True)
-        return input_validator.generate_control_treatments(list(features))
+        return input_validator.generate_control_treatments(list(features), method_name)
+
+    def _evaluate_features_if_ready(self, matching_key, bucketing_key, features, attributes=None):
+        if not self.ready:
+            return {
+                feature: {
+                    'treatment': CONTROL,
+                    'configurations': None,
+                    'impression': {'label': Label.NOT_READY, 'change_number': None}
+                }
+                for feature in features
+            }
+
+        return self._evaluator.evaluate_features(
+            features,
+            matching_key,
+            bucketing_key,
+            attributes
+        )
+
+    def get_treatment_with_config(self, key, feature, attributes=None):
+        """
+        Get the treatment and config for a feature and key, with optional dictionary of attributes.
+
+        This method never raises an exception. If there's a problem, the appropriate log message
+        will be generated and the method will return the CONTROL treatment.
+
+        :param key: The key for which to get the treatment
+        :type key: str
+        :param feature: The name of the feature for which to get the treatment
+        :type feature: str
+        :param attributes: An optional dictionary of attributes
+        :type attributes: dict
+        :return: The treatment for the key and feature
+        :rtype: tuple(str, str)
+        """
+        return self._make_evaluation(key, feature, attributes, 'get_treatment_with_config',
+                                     self._METRIC_GET_TREATMENT_WITH_CONFIG)
+
+    def get_treatment(self, key, feature, attributes=None):
+        """
+        Get the treatment for a feature and key, with an optional dictionary of attributes.
+
+        This method never raises an exception. If there's a problem, the appropriate log message
+        will be generated and the method will return the CONTROL treatment.
+
+        :param key: The key for which to get the treatment
+        :type key: str
+        :param feature: The name of the feature for which to get the treatment
+        :type feature: str
+        :param attributes: An optional dictionary of attributes
+        :type attributes: dict
+        :return: The treatment for the key and feature
+        :rtype: str
+        """
+        treatment, _ = self._make_evaluation(key, feature, attributes, 'get_treatment',
+                                             self._METRIC_GET_TREATMENT)
+        return treatment
+
+    def get_treatments_with_config(self, key, features, attributes=None):
+        """
+        Evaluate multiple features and return a dict with feature -> (treatment, config).
+
+        Get the treatments for a list of features considering a key, with an optional dictionary of
+        attributes. This method never raises an exception. If there's a problem, the appropriate
+        log message will be generated and the method will return the CONTROL treatment.
+        :param key: The key for which to get the treatment
+        :type key: str
+        :param features: Array of the names of the features for which to get the treatment
+        :type feature: list
+        :param attributes: An optional dictionary of attributes
+        :type attributes: dict
+        :return: Dictionary with the result of all the features provided
+        :rtype: dict
+        """
+        return self._make_evaluations(key, features, attributes, 'get_treatments_with_config',
+                                      self._METRIC_GET_TREATMENTS_WITH_CONFIG)
 
     def get_treatments(self, key, features, attributes=None):
         """
@@ -308,7 +321,8 @@ def get_treatments(self, key, features, attributes=None):
         :return: Dictionary with the result of all the features provided
         :rtype: dict
         """
-        with_config = self.get_treatments_with_config(key, features, attributes)
+        with_config = self._make_evaluations(key, features, attributes, 'get_treatments',
+                                             self._METRIC_GET_TREATMENTS)
         return {feature: result[0] for (feature, result) in six.iteritems(with_config)}
 
     def _build_impression(  # pylint: disable=too-many-arguments
@@ -346,10 +360,7 @@ def _record_stats(self, impressions, start, operation):
         """
         try:
             end = int(round(time.time() * 1000))
-            if operation == self._METRIC_GET_TREATMENT:
-                self._impressions_storage.put([impressions])
-            else:
-                self._impressions_storage.put(impressions)
+            self._impressions_storage.put(impressions)
             self._telemetry_storage.inc_latency(operation, get_latency_bucket_index(end - start))
         except Exception:  # pylint: disable=broad-except
             self._logger.error('Error recording impressions and metrics')