Refactor Redis client instantiation (fixes #567)

CHANGELOG.rst

@@ -44,6 +44,7 @@ This document describes changes between each past release.
 - Added a simple end-to-end test on a *Cliquet* sample application, using
   `Loads <http://github.com/loads/>`_ (fixes #512)
 - Switched to SQLAlchemy sessions instead of raw connections and cursors (#510)
+- Refactor Redis clients instantiation to avoid repeated defaults (#567, #568)
 
 
 2.10.2 (2015-11-10)

cliquet/cache/redis.py

@@ -1,10 +1,7 @@
 from __future__ import absolute_import
 
-import redis
-from six.moves.urllib import parse as urlparse
-
 from cliquet.cache import CacheBase
-from cliquet.storage.redis import wrap_redis_error
+from cliquet.storage.redis import wrap_redis_error, create_from_config
 from cliquet.utils import json
 
 
@@ -26,10 +23,13 @@ class Cache(CacheBase):
     :noindex:
     """
 
-    def __init__(self, *args, **kwargs):
+    def __init__(self, client, *args, **kwargs):
         super(Cache, self).__init__(*args, **kwargs)
-        connection_pool = redis.BlockingConnectionPool(**kwargs)
-        self._client = redis.StrictRedis(connection_pool=connection_pool)
+        self._client = client
+
+    @property
+    def settings(self):
+        return dict(self._client.connection_pool.connection_kwargs)
 
     def initialize_schema(self):
         # Nothing to do.
@@ -68,13 +68,5 @@ def delete(self, key):
 
 
 def load_from_config(config):
-    settings = config.get_settings()
-    uri = settings['cache_url']
-    uri = urlparse.urlparse(uri)
-    pool_size = int(settings['cache_pool_size'])
-
-    return Cache(max_connections=pool_size,
-                 host=uri.hostname or 'localhost',
-                 port=uri.port or 6379,
-                 password=uri.password or None,
-                 db=int(uri.path[1:]) if uri.path else 0)
+    client = create_from_config(config, prefix='cache_')
+    return Cache(client)

cliquet/permission/redis.py

@@ -1,11 +1,9 @@
 from __future__ import absolute_import
 
-import redis
 from collections import defaultdict
-from six.moves.urllib import parse as urlparse
 
 from cliquet.permission import PermissionBase
-from cliquet.storage.redis import wrap_redis_error
+from cliquet.storage.redis import create_from_config, wrap_redis_error
 
 
 class Permission(PermissionBase):
@@ -26,10 +24,13 @@ class Permission(PermissionBase):
     :noindex:
     """
 
-    def __init__(self, *args, **kwargs):
+    def __init__(self, client, *args, **kwargs):
         super(Permission, self).__init__(*args, **kwargs)
-        connection_pool = redis.BlockingConnectionPool(**kwargs)
-        self._client = redis.StrictRedis(connection_pool=connection_pool)
+        self._client = client
+
+    @property
+    def settings(self):
+        return dict(self._client.connection_pool.connection_kwargs)
 
     def initialize_schema(self):
         # Nothing to do.
@@ -163,13 +164,5 @@ def delete_object_permissions(self, *object_id_list):
 
 
 def load_from_config(config):
-    settings = config.get_settings()
-    uri = settings['permission_url']
-    uri = urlparse.urlparse(uri)
-    pool_size = int(settings['permission_pool_size'])
-
-    return Permission(max_connections=pool_size,
-                      host=uri.hostname or 'localhost',
-                      port=uri.port or 6379,
-                      password=uri.password or None,
-                      db=int(uri.path[1:]) if uri.path else 0)
+    client = create_from_config(config, prefix='permission_')
+    return Permission(client)

cliquet/storage/redis.py

@@ -21,6 +21,24 @@ def wrapped(*args, **kwargs):
     return wrapped
 
 
+def create_from_config(config, prefix=''):
+    """Redis client instantiation from settings.
+    """
+    settings = config.get_settings()
+    uri = settings[prefix + 'url']
+    uri = urlparse.urlparse(uri)
+    pool_size = int(settings[prefix + 'pool_size'])
+    kwargs = {
+        "max_connections": pool_size,
+        "host": uri.hostname or 'localhost',
+        "port": uri.port or 6379,
+        "password": uri.password or None,
+        "db": int(uri.path[1:]) if uri.path else 0
+    }
+    connection_pool = redis.BlockingConnectionPool(**kwargs)
+    return redis.StrictRedis(connection_pool=connection_pool)
+
+
 class Storage(MemoryBasedStorage):
     """Storage backend implementation using Redis.
 
@@ -42,10 +60,13 @@ class Storage(MemoryBasedStorage):
         cliquet.storage_pool_size = 50
     """
 
-    def __init__(self, *args, **kwargs):
+    def __init__(self, client, *args, **kwargs):
         super(Storage, self).__init__(*args, **kwargs)
-        connection_pool = redis.BlockingConnectionPool(**kwargs)
-        self._client = redis.StrictRedis(connection_pool=connection_pool)
+        self._client = client
+
+    @property
+    def settings(self):
+        return dict(self._client.connection_pool.connection_kwargs)
 
     def _encode(self, record):
         return utils.json.dumps(record)
@@ -288,13 +309,5 @@ def get_all(self, collection_id, parent_id, filters=None, sorting=None,
 
 
 def load_from_config(config):
-    settings = config.get_settings()
-    uri = settings['storage_url']
-    uri = urlparse.urlparse(uri)
-    pool_size = int(settings['storage_pool_size'])
-
-    return Storage(max_connections=pool_size,
-                   host=uri.hostname or 'localhost',
-                   port=uri.port or 6379,
-                   password=uri.password or None,
-                   db=int(uri.path[1:]) if uri.path else 0)
+    client = create_from_config(config, prefix='storage_')
+    return Storage(client)

cliquet/tests/test_cache.py

@@ -176,7 +176,7 @@ def test_config_is_taken_in_account(self):
         config.add_settings({'cache_url': 'redis://:secret@peer.loc:4444/7'})
         backend = self.backend.load_from_config(config)
         self.assertDictEqual(
-            backend._client.connection_pool.connection_kwargs,
+            backend.settings,
             {'host': 'peer.loc', 'password': 'secret', 'db': 7, 'port': 4444})
 
 

cliquet/tests/test_listeners.py

@@ -1,16 +1,17 @@
 # -*- coding: utf-8 -*-
-import mock
-from pyramid import testing
-import uuid
-import redis
 import json
-from datetime import datetime
+import uuid
 from contextlib import contextmanager
+from datetime import datetime
+
+import mock
+from pyramid import testing
 
 from cliquet import initialization
 from cliquet.events import ResourceChanged, ACTIONS
 from cliquet.listeners import ListenerBase
-from .support import unittest
+from cliquet.storage.redis import create_from_config
+from cliquet.tests.support import unittest
 
 
 class ListenerSetupTest(unittest.TestCase):
@@ -70,9 +71,9 @@ class ListenerCalledTest(unittest.TestCase):
 
     def setUp(self):
         self.config = testing.setUp()
-        pool = redis.BlockingConnectionPool(max_connections=1,
-                                            host='localhost', port=6379, db=0)
-        self._redis = redis.StrictRedis(connection_pool=pool)
+        self.config.add_settings({'events_pool_size': 1,
+                                  'events_url': 'redis://localhost:6379/0'})
+        self._redis = create_from_config(self.config, prefix='events_')
         self._size = 0
 
     def _save_redis(self):

cliquet/tests/test_permission.py

@@ -456,7 +456,7 @@ def test_config_is_taken_in_account(self):
         config.add_settings({'permission_url': 'redis://:pass@db.loc:1234/5'})
         backend = self.backend.load_from_config(config)
         self.assertDictEqual(
-            backend._client.connection_pool.connection_kwargs,
+            backend.settings,
             {'host': 'db.loc', 'password': 'pass', 'db': 5, 'port': 1234})
 
 

cliquet/tests/test_storage.py

@@ -977,7 +977,7 @@ def test_config_is_taken_in_account(self):
         config.add_settings({'storage_url': 'redis://:blah@store.loc:7777/6'})
         backend = self.backend.load_from_config(config)
         self.assertDictEqual(
-            backend._client.connection_pool.connection_kwargs,
+            backend.settings,
             {'host': 'store.loc', 'password': 'blah', 'db': 6, 'port': 7777})
 
     def test_backend_error_provides_original_exception(self):