Browse Source

Fixed #20892 -- Allowed configuring memcached client using OPTIONS.

Previously, the MemcachedCache backend ignored `OPTIONS` and
PyLibMCCache used them to set pylibmc behaviors. Both backends now
pass `OPTIONS` as keyword arguments to the client constructors.
Ed Morley 8 years ago
parent
commit
65ec8fa8ca

+ 1 - 0
AUTHORS

@@ -224,6 +224,7 @@ answer newbie questions, and generally made Django that much better:
     Doug Napoleone <doug@dougma.com>
     dready <wil@mojipage.com>
     dusk@woofle.net
+    Ed Morley <https://github.com/edmorley>
     eibaan@gmail.com
     Emil Stenström <em@kth.se>
     enlight

+ 27 - 8
django/core/cache/backends/memcached.py

@@ -2,9 +2,11 @@
 
 import pickle
 import time
+import warnings
 
 from django.core.cache.backends.base import DEFAULT_TIMEOUT, BaseCache
 from django.utils import six
+from django.utils.deprecation import RemovedInDjango21Warning
 from django.utils.encoding import force_str
 from django.utils.functional import cached_property
 
@@ -24,7 +26,7 @@ class BaseMemcachedCache(BaseCache):
         self.LibraryValueNotFoundException = value_not_found_exception
 
         self._lib = library
-        self._options = params.get('OPTIONS')
+        self._options = params.get('OPTIONS') or {}
 
     @property
     def _cache(self):
@@ -32,7 +34,7 @@ class BaseMemcachedCache(BaseCache):
         Implements transparent thread-safe access to a memcached client.
         """
         if getattr(self, '_client', None) is None:
-            self._client = self._lib.Client(self._servers)
+            self._client = self._lib.Client(self._servers, **self._options)
 
         return self._client
 
@@ -163,7 +165,9 @@ class MemcachedCache(BaseMemcachedCache):
     @property
     def _cache(self):
         if getattr(self, '_client', None) is None:
-            self._client = self._lib.Client(self._servers, pickleProtocol=pickle.HIGHEST_PROTOCOL)
+            client_kwargs = dict(pickleProtocol=pickle.HIGHEST_PROTOCOL)
+            client_kwargs.update(self._options)
+            self._client = self._lib.Client(self._servers, **client_kwargs)
         return self._client
 
 
@@ -175,10 +179,25 @@ class PyLibMCCache(BaseMemcachedCache):
                                            library=pylibmc,
                                            value_not_found_exception=pylibmc.NotFound)
 
+        # The contents of `OPTIONS` was formerly only used to set the behaviors
+        # attribute, but is now passed directly to the Client constructor. As such,
+        # any options that don't match a valid keyword argument are removed and set
+        # under the `behaviors` key instead, to maintain backwards compatibility.
+        legacy_behaviors = {}
+        for option in list(self._options):
+            if option not in ('behaviors', 'binary', 'username', 'password'):
+                warnings.warn(
+                    "Specifying pylibmc cache behaviors as a top-level property "
+                    "within `OPTIONS` is deprecated. Move `%s` into a dict named "
+                    "`behaviors` inside `OPTIONS` instead." % option,
+                    RemovedInDjango21Warning,
+                    stacklevel=2,
+                )
+                legacy_behaviors[option] = self._options.pop(option)
+
+        if legacy_behaviors:
+            self._options.setdefault('behaviors', {}).update(legacy_behaviors)
+
     @cached_property
     def _cache(self):
-        client = self._lib.Client(self._servers)
-        if self._options:
-            client.behaviors = self._options
-
-        return client
+        return self._lib.Client(self._servers, **self._options)

+ 3 - 0
docs/internals/deprecation.txt

@@ -27,6 +27,9 @@ details on these changes.
 
 * ``django.utils.translation.string_concat()`` will be removed.
 
+* ``django.core.cache.backends.memcached.PyLibMCCache`` will no longer support
+  passing ``pylibmc`` behavior settings as top-level attributes of ``OPTIONS``.
+
 .. _deprecation-removed-in-2.0:
 
 2.0

+ 8 - 1
docs/releases/1.11.txt

@@ -168,7 +168,10 @@ Minor features
 Cache
 ~~~~~
 
-* ...
+* Memcached backends now pass the contents of :setting:`OPTIONS <CACHES-OPTIONS>`
+  as keyword arguments to the client constructors, allowing for more advanced
+  control of client behavior. See the :ref:`cache arguments <cache_arguments>`
+  documentation for examples.
 
 CSRF
 ~~~~
@@ -490,3 +493,7 @@ Miscellaneous
 * ``django.utils.translation.string_concat()`` is deprecated in
   favor of :func:`django.utils.text.format_lazy`. ``string_concat(*strings)``
   can be replaced by ``format_lazy('{}' * len(strings), *strings)``.
+
+* For the ``PyLibMCCache`` cache backend, passing ``pylibmc`` behavior settings
+  as top-level attributes of ``OPTIONS`` is deprecated. Set them under a
+  ``behaviors`` key within ``OPTIONS`` instead.

+ 42 - 2
docs/topics/cache.txt

@@ -403,6 +403,10 @@ behavior. These arguments are provided as additional keys in the
     On some backends (``database`` in particular) this makes culling *much*
     faster at the expense of more cache misses.
 
+  Memcached backends pass the contents of :setting:`OPTIONS <CACHES-OPTIONS>`
+  as keyword arguments to the client constructors, allowing for more advanced
+  control of client behavior. For example usage, see below.
+
 * :setting:`KEY_PREFIX <CACHES-KEY_PREFIX>`: A string that will be
   automatically included (prepended by default) to all cache keys
   used by the Django server.
@@ -437,8 +441,44 @@ of 60 seconds, and a maximum capacity of 1000 items::
         }
     }
 
-Invalid arguments are silently ignored, as are invalid values of known
-arguments.
+Here's an example configuration for a ``python-memcached`` based backend with
+an object size limit of 2MB::
+
+    CACHES = {
+        'default': {
+            'BACKEND': 'django.core.cache.backends.memcached.MemcachedCache',
+            'LOCATION': '127.0.0.1:11211',
+            'OPTIONS': {
+                'server_max_value_length': 1024 * 1024 * 2,
+            }
+        }
+    }
+
+Here's an example configuration for a ``pylibmc`` based backend that enables
+the binary protocol, SASL authentication, and the ``ketama`` behavior mode::
+
+    CACHES = {
+        'default': {
+            'BACKEND': 'django.core.cache.backends.memcached.PyLibMCCache',
+            'LOCATION': '127.0.0.1:11211',
+            'OPTIONS': {
+                'binary': True,
+                'username': 'user',
+                'password': 'pass',
+                'behaviors': {
+                    'ketama': True,
+                }
+            }
+        }
+    }
+
+.. versionchanged:: 1.11
+
+    Memcached backends can now be configured using ``OPTIONS``.
+
+    In older versions, you could pass ``pylibmc`` behavior settings directly
+    inside ``OPTIONS``. This is deprecated in favor of setting them under a
+    ``behaviors`` key within ``OPTIONS`` instead.
 
 .. _the-per-site-cache:
 

+ 39 - 0
tests/cache/tests.py

@@ -40,6 +40,7 @@ from django.utils.cache import (
     get_cache_key, learn_cache_key, patch_cache_control,
     patch_response_headers, patch_vary_headers,
 )
+from django.utils.deprecation import RemovedInDjango21Warning
 from django.utils.encoding import force_text
 from django.views.decorators.cache import cache_page
 
@@ -1241,6 +1242,14 @@ class MemcachedCacheTests(BaseMemcachedTests, TestCase):
         for cache_key in settings.CACHES:
             self.assertEqual(caches[cache_key]._cache.pickleProtocol, pickle.HIGHEST_PROTOCOL)
 
+    @override_settings(CACHES=caches_setting_for_tests(
+        base=MemcachedCache_params,
+        exclude=memcached_excluded_caches,
+        OPTIONS={'server_max_value_length': 9999},
+    ))
+    def test_memcached_options(self):
+        self.assertEqual(cache._cache.server_max_value_length, 9999)
+
 
 @unittest.skipUnless(PyLibMCCache_params, "PyLibMCCache backend not configured")
 @override_settings(CACHES=caches_setting_for_tests(
@@ -1259,6 +1268,36 @@ class PyLibMCCacheTests(BaseMemcachedTests, TestCase):
     def test_invalid_key_characters(self):
         pass
 
+    @override_settings(CACHES=caches_setting_for_tests(
+        base=PyLibMCCache_params,
+        exclude=memcached_excluded_caches,
+        OPTIONS={
+            'binary': True,
+            'behaviors': {'tcp_nodelay': True},
+        },
+    ))
+    def test_pylibmc_options(self):
+        self.assertTrue(cache._cache.binary)
+        self.assertEqual(cache._cache.behaviors['tcp_nodelay'], int(True))
+
+    @override_settings(CACHES=caches_setting_for_tests(
+        base=PyLibMCCache_params,
+        exclude=memcached_excluded_caches,
+        OPTIONS={'tcp_nodelay': True},
+    ))
+    def test_pylibmc_legacy_options(self):
+        deprecation_message = (
+            "Specifying pylibmc cache behaviors as a top-level property "
+            "within `OPTIONS` is deprecated. Move `tcp_nodelay` into a dict named "
+            "`behaviors` inside `OPTIONS` instead."
+        )
+        with warnings.catch_warnings(record=True) as warns:
+            warnings.simplefilter("always")
+            self.assertEqual(cache._cache.behaviors['tcp_nodelay'], int(True))
+        self.assertEqual(len(warns), 1)
+        self.assertIsInstance(warns[0].message, RemovedInDjango21Warning)
+        self.assertEqual(str(warns[0].message), deprecation_message)
+
 
 @override_settings(CACHES=caches_setting_for_tests(
     BACKEND='django.core.cache.backends.filebased.FileBasedCache',