splitio · mredolatti · Apr 23, 2019 · Apr 22, 2019 · Apr 22, 2019 · Apr 22, 2019
diff --git a/splitio/client/client.py b/splitio/client/client.py
@@ -236,20 +236,18 @@ def get_treatments_with_config(self, key, features, attributes=None):
                                    'feature ' + feature + ' returning CONTROL.')
                 treatments[feature] = CONTROL, None
                 self._logger.debug('Error: ', exc_info=True)
-                import traceback
-                traceback.print_exc()
                 continue
 
-            # Register impressions
-            try:
-                if bulk_impressions:
-                    self._record_stats(bulk_impressions, start, self._METRIC_GET_TREATMENTS)
-                    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.debug('Error: ', exc_info=True)
+        # Register impressions
+        try:
+            if bulk_impressions:
+                self._record_stats(bulk_impressions, start, self._METRIC_GET_TREATMENTS)
+                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.debug('Error: ', exc_info=True)
 
         return treatments
 

diff --git a/splitio/client/config.py b/splitio/client/config.py
@@ -39,5 +39,7 @@
     'redisSslCertReqs': None,
     'redisSslCaCerts': None,
     'redisMaxConnections': None,
+    'machineName': None,
+    'machineIp': None,
     'splitFile': os.path.join(os.path.expanduser('~'), '.split')
 }
diff --git a/splitio/client/factory.py b/splitio/client/factory.py
@@ -312,7 +312,7 @@ def _build_redis_factory(config):
     """Build and return a split factory with redis-based storage."""
     cfg = DEFAULT_CONFIG.copy()
     cfg.update(config)
-    sdk_metadata = util.get_metadata()
+    sdk_metadata = util.get_metadata(config)
     redis_adapter = redis.build(config)
     storages = {
         'splits': RedisSplitStorage(redis_adapter),

diff --git a/splitio/client/util.py b/splitio/client/util.py
@@ -11,8 +11,6 @@
 )
 
 
-
-
 def _get_ip():
     sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
     try:
@@ -30,11 +28,21 @@ def _get_hostname(ip_address):
     return 'unknown' if ip_address == 'unknown' else 'ip-' + ip_address.replace('.', '-')
 
 
-def get_metadata(*args, **kwargs):
-    """Gather SDK metadata and return a tuple with such info."""
+def get_metadata(config):
+    """
+    Gather SDK metadata and return a tuple with such info.
+
+    :param config: User supplied config augmented with defaults.
+    :type config: dict
+
+    :return: SDK Metadata information.
+    :rtype: SdkMetadata
+    """
     version = 'python-%s' % __version__
-    ip_address = _get_ip()
-    hostname = _get_hostname(ip_address)
+    ip_from_config = config.get('machineIp')
+    machine_from_config = config.get('machineName')
+    ip_address = ip_from_config if ip_from_config is not None else _get_ip()
+    hostname = machine_from_config if machine_from_config is not None else _get_hostname(ip_address)
     return SdkMetadata(version, hostname, ip_address)
 
 
@@ -48,9 +56,12 @@ def get_calls(classes_filter=None):
     :return: list of callers ordered by most recent first.
     :rtype: list(tuple(str, str))
     """
-    return [
-        inspect.getframeinfo(frame[0]).function
-        for frame in inspect.stack()
-        if classes_filter is None
-        or 'self' in frame[0].f_locals and frame[0].f_locals['self'].__class__.__name__ in classes_filter  #pylint: disable=line-too-long
-    ]
+    try:
+        return [
+            inspect.getframeinfo(frame[0]).function
+            for frame in inspect.stack()
+            if classes_filter is None
+            or 'self' in frame[0].f_locals and frame[0].f_locals['self'].__class__.__name__ in classes_filter  #pylint: disable=line-too-long
+        ]
+    except Exception:  #pylint: disable=broad-except
+        return []
diff --git a/splitio/storage/adapters/redis.py b/splitio/storage/adapters/redis.py
@@ -279,6 +279,13 @@ def ttl(self, key):
         except RedisError as exc:
             raise_from(RedisAdapterException('Error executing ttl operation'), exc)
 
+    def lpop(self, key):
+        """Mimic original redis function but using user custom prefix."""
+        try:
+            return self._decorated.lpop(self._add_prefix(key))
+        except RedisError as exc:
+            raise_from(RedisAdapterException('Error executing lpop operation'), exc)
+
 
 def _build_default_client(config):  #pylint: disable=too-many-locals
     """

diff --git a/splitio/storage/inmemmory.py b/splitio/storage/inmemmory.py
@@ -308,7 +308,7 @@ def put(self, events):
             if self._queue_full_hook is not None and callable(self._queue_full_hook):
                 self._queue_full_hook()
             self._logger.warning(
-                'Impressions queue is full, failing to add more events. \n'
+                'Events queue is full, failing to add more events. \n'
                 'Consider increasing parameter `impressionsQueueSize` in configuration'
             )
             return False

diff --git a/splitio/storage/redis.py b/splitio/storage/redis.py
@@ -110,6 +110,7 @@ def get_split_names(self):
         """
         try:
             keys = self._redis.keys(self._get_key('*'))
+            for key in keys:
             return [key.replace(self._get_key(''), '') for key in keys]
         except RedisAdapterException:
             self._logger.error('Error fetching split names from storage')
@@ -129,7 +130,6 @@ def get_all_splits(self):
             raw_splits = self._redis.mget(keys)
             for raw in raw_splits:
                 try:
-                    print(raw, type(raw))
                     to_return.append(splits.from_raw(json.loads(raw)))
                 except ValueError:
                     self._logger.error('Could not parse split. Skipping')
@@ -224,7 +224,6 @@ def get_change_number(self, segment_name):
         """
         try:
             stored_value = self._redis.get(self._get_till_key(segment_name))
-            print('aaa', stored_value)
             return json.loads(stored_value) if stored_value is not None else None
         except RedisAdapterException:
             self._logger.error('Error fetching segment change number from storage')

diff --git a/tests/client/files/file1.split b/tests/client/files/file1.split
@@ -1,14 +1,2 @@
-events_write_es on
-events_routing sqs
-impressions_routing sqs
-workspaces_v1 on
-create_org_with_workspace on
-sqs_events_processing on
-sqs_impressions_processing on
-sqs_events_fetch on
-sqs_impressions_fetch off
-sqs_impressions_fetch_period 700
-sqs_impressions_fetch_threads 10
-sqs_events_fetch_period 500
-sqs_events_fetch_threads 5
-
+split1 on
+split2 off
diff --git a/tests/client/test_client.py b/tests/client/test_client.py
@@ -1,13 +1,18 @@
 """SDK main client test module."""
 #pylint: disable=no-self-use,protected-access
 
+import json
+import os
 from splitio.client.client import Client
 from splitio.client.factory import SplitFactory
 from splitio.engine.evaluator import Evaluator
 from splitio.models.impressions import Impression
 from splitio.models.events import Event
 from splitio.storage import EventStorage, ImpressionStorage, SegmentStorage, SplitStorage, \
     TelemetryStorage
+from splitio.storage.inmemmory import InMemorySplitStorage, InMemorySegmentStorage, \
+    InMemoryImpressionStorage, InMemoryTelemetryStorage, InMemoryEventStorage
+from splitio.models import splits, segments
 
 class ClientTests(object):  #pylint: disable=too-few-public-methods
     """Split client test cases."""
@@ -197,7 +202,7 @@ def _raise(*_):
             raise Exception('something')
         client._evaluator.evaluate_treatment.side_effect = _raise
         assert client.get_treatments('key', ['f1', 'f2']) == {'f1': 'control', 'f2': 'control'}
-        assert len(telemetry_storage.inc_latency.mock_calls) == 2
+        assert len(telemetry_storage.inc_latency.mock_calls) == 1
 
     def test_get_treatments_with_config(self, mocker):
         """Test get_treatment execution paths."""
@@ -265,7 +270,7 @@ def _raise(*_):
             'f1': ('control', None),
             'f2': ('control', None)
         }
-        assert len(telemetry_storage.inc_latency.mock_calls) == 2
+        assert len(telemetry_storage.inc_latency.mock_calls) == 1
 
     def test_destroy(self, mocker):
         """Test that destroy/destroyed calls are forwarded to the factory."""

diff --git a/tests/client/test_localhost.py b/tests/client/test_localhost.py
@@ -4,7 +4,6 @@
 import os
 import tempfile
 
-from splitio.client.factory import get_factory
 from splitio.client import localhost
 from splitio.models.splits import Split
 from splitio.models.grammar.matchers import AllKeysMatcher
@@ -106,18 +105,15 @@ def test_make_whitelist_condition(self):
 
     def test_parse_legacy_file(self):
         """Test that aprsing a legacy file works."""
-        with tempfile.NamedTemporaryFile() as temp_flo:
-            temp_flo.write('split1 on\n')
-            temp_flo.write('split2 off\n')
-            temp_flo.flush()
-            splits = localhost.LocalhostSplitSynchronizationTask._read_splits_from_legacy_file(temp_flo.name)
-            assert len(splits) == 2
-            for split in splits.values():
-                assert isinstance(split, Split)
-            assert splits['split1'].name == 'split1'
-            assert splits['split2'].name == 'split2'
-            assert isinstance(splits['split1'].conditions[0].matchers[0], AllKeysMatcher)
-            assert isinstance(splits['split2'].conditions[0].matchers[0], AllKeysMatcher)
+        filename = os.path.join(os.path.dirname(__file__), 'files', 'file1.split')
+        splits = localhost.LocalhostSplitSynchronizationTask._read_splits_from_legacy_file(filename)
+        assert len(splits) == 2
+        for split in splits.values():
+            assert isinstance(split, Split)
+        assert splits['split1'].name == 'split1'
+        assert splits['split2'].name == 'split2'
+        assert isinstance(splits['split1'].conditions[0].matchers[0], AllKeysMatcher)
+        assert isinstance(splits['split2'].conditions[0].matchers[0], AllKeysMatcher)
 
     def test_parse_yaml_file(self):
         """Test that parsing a yaml file works."""
@@ -195,30 +191,3 @@ def test_update_splits(self, mocker):
         task._update_splits()
         assert parse_legacy.mock_calls == [mocker.call('yaml')]
         assert parse_yaml.mock_calls == []
-
-    def test_localhost_e2e(self):
-        """Instantiate a client with a YAML file and issue get_treatment() calls."""
-        filename = os.path.join(os.path.dirname(__file__), 'files', 'file2.yaml')
-        factory = get_factory('localhost', config={'splitFile': filename})
-        client = factory.client()
-        assert client.get_treatment_with_config('key', 'my_feature') == ('on', '{"desc" : "this applies only to ON treatment"}')
-        assert client.get_treatment_with_config('only_key', 'my_feature') == (
-            'off', '{"desc" : "this applies only to OFF and only for only_key. The rest will receive ON"}'
-        )
-        assert client.get_treatment_with_config('another_key', 'my_feature') == ('control', None)
-        assert client.get_treatment_with_config('key2', 'other_feature') == ('on', None)
-        assert client.get_treatment_with_config('key3', 'other_feature') == ('on', None)
-        assert client.get_treatment_with_config('some_key', 'other_feature_2') == ('on', None)
-        assert client.get_treatment_with_config('key_whitelist', 'other_feature_3') == ('on', None)
-        assert client.get_treatment_with_config('any_other_key', 'other_feature_3') == ('off', None)
-
-        manager = factory.manager()
-        assert manager.split('my_feature').configs == {
-            'on': '{"desc" : "this applies only to ON treatment"}',
-            'off': '{"desc" : "this applies only to OFF and only for only_key. The rest will receive ON"}'
-        }
-        assert manager.split('other_feature').configs == {}
-        assert manager.split('other_feature_2').configs == {}
-        assert manager.split('other_feature_3').configs == {}
-
-
diff --git a/tests/client/test_utils.py b/tests/client/test_utils.py
@@ -0,0 +1,32 @@
+"""Split client utilities test module."""
+#pylint: disable=no-self-use,too-few-public-methods
+
+from splitio.client import util, config
+from splitio.version import __version__
+
+class ClientUtilsTests(object):
+    """Client utilities test cases."""
+
+    def test_get_metadata(self, mocker):
+        """Test the get_metadata function."""
+        get_ip_mock = mocker.Mock()
+        get_host_mock = mocker.Mock()
+        mocker.patch('splitio.client.util._get_ip', new=get_ip_mock)
+        mocker.patch('splitio.client.util._get_hostname', new=get_host_mock)
+
+        meta = util.get_metadata({'machineIp': 'some_ip', 'machineName': 'some_machine_name'})
+        assert get_ip_mock.mock_calls == []
+        assert get_host_mock.mock_calls == []
+        assert meta.instance_ip == 'some_ip'
+        assert meta.instance_name == 'some_machine_name'
+        assert meta.sdk_version == 'python-' + __version__
+
+        meta = util.get_metadata(config.DEFAULT_CONFIG)
+        assert get_ip_mock.mock_calls == [mocker.call()]
+        assert get_host_mock.mock_calls == [mocker.call(mocker.ANY)]
+
+        get_ip_mock.reset_mock()
+        get_host_mock.reset_mock()
+        meta = util.get_metadata({})
+        assert get_ip_mock.mock_calls == [mocker.call()]
+        assert get_host_mock.mock_calls == [mocker.call(mocker.ANY)]
diff --git a/tests/integration/files/file2.yaml b/tests/integration/files/file2.yaml
@@ -0,0 +1,18 @@
+- my_feature:
+    treatment: "on"
+    keys: "key"
+    config: "{\"desc\" : \"this applies only to ON treatment\"}"
+- other_feature_3:
+    treatment: "off"
+- my_feature:
+    treatment: "off"
+    keys: "only_key"
+    config: "{\"desc\" : \"this applies only to OFF and only for only_key. The rest will receive ON\"}"
+- other_feature_3:
+    treatment: "on"
+    keys: "key_whitelist"
+- other_feature:
+    treatment: "on"
+    keys: ["key2","key3"]
+- other_feature_2:
+    treatment: "on"
diff --git a/tests/integration/files/segmentEmployeesChanges.json b/tests/integration/files/segmentEmployeesChanges.json
@@ -0,0 +1,10 @@
+{
+  "name": "employees",
+  "added": [
+    "employee_3",
+    "employee_1"
+  ],
+  "removed": [],
+  "since": -1,
+  "till": 1457474612832
+}
diff --git a/tests/integration/files/segmentHumanBeignsChanges.json b/tests/integration/files/segmentHumanBeignsChanges.json
@@ -0,0 +1,10 @@
+{
+  "name": "human_beigns",
+  "added": [
+    "user1",
+    "user3"
+  ],
+  "removed": [],
+  "since": -1,
+  "till": 1457102183278
+}