From 65ec8fa8ca56a5378345375e1079025c96d0b833 Mon Sep 17 00:00:00 2001 From: Ed Morley Date: Wed, 31 Aug 2016 13:12:40 +0100 Subject: [PATCH] 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. --- AUTHORS | 1 + django/core/cache/backends/memcached.py | 35 +++++++++++++++----- docs/internals/deprecation.txt | 3 ++ docs/releases/1.11.txt | 9 ++++- docs/topics/cache.txt | 44 +++++++++++++++++++++++-- tests/cache/tests.py | 39 ++++++++++++++++++++++ 6 files changed, 120 insertions(+), 11 deletions(-) diff --git a/AUTHORS b/AUTHORS index 5c6614a951..e31e67c943 100644 --- a/AUTHORS +++ b/AUTHORS @@ -224,6 +224,7 @@ answer newbie questions, and generally made Django that much better: Doug Napoleone dready dusk@woofle.net + Ed Morley eibaan@gmail.com Emil Stenström enlight diff --git a/django/core/cache/backends/memcached.py b/django/core/cache/backends/memcached.py index ee6b3b7712..2ca9f663d7 100644 --- a/django/core/cache/backends/memcached.py +++ b/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) diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 7aa58e4ee7..b983ae1ace 100644 --- a/docs/internals/deprecation.txt +++ b/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 diff --git a/docs/releases/1.11.txt b/docs/releases/1.11.txt index 3953ba917c..d6a59c8ab9 100644 --- a/docs/releases/1.11.txt +++ b/docs/releases/1.11.txt @@ -168,7 +168,10 @@ Minor features Cache ~~~~~ -* ... +* Memcached backends now pass the contents of :setting:`OPTIONS ` + as keyword arguments to the client constructors, allowing for more advanced + control of client behavior. See the :ref:`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. diff --git a/docs/topics/cache.txt b/docs/topics/cache.txt index 50b4be95b9..1730ef6294 100644 --- a/docs/topics/cache.txt +++ b/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 ` + as keyword arguments to the client constructors, allowing for more advanced + control of client behavior. For example usage, see below. + * :setting:`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: diff --git a/tests/cache/tests.py b/tests/cache/tests.py index 1de1f01490..c5fb42f476 100644 --- a/tests/cache/tests.py +++ b/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',