Moves dead_retry and socket_timeout into the MemcachedBackend class

Moved the :paramref:`.MemcacheArgs.dead_retry` argument and the
:paramref:`.MemcacheArgs.socket_timeout` argument which were
erroneously added to the "set_parameters",
where they have no effect, to be part of the Memcached connection
arguments :paramref:`.MemcachedBackend.dead_retry`,
:paramref:`.MemcachedBackend.socket_timeout`.

Indeed, In my previous patch [1] I proposed to add the ``dead_retry`` and ``socket_timeout`` params to the ``MemcacheArgs`` class. I was wrong. My goal was to pass these parameters to the client during its initialization to set the memcached client dead_retry and socket_timeout arguments [2].

By using the MemcacheArgs they are passed to the method calls which is not what it was requested in the feature request [3]. I misunderstood the goal of this class (MemcacheArgs).

The ``MemcacheArgs`` class is only inherited by the ``MemcachedBackend`` class and the ``PylibmcBackend`` class. Both libraries doesn't support ``dead_retry`` and ``socket_timeout`` in their methods related to the memcache API commands (set, get, set_multi, etc), so for this reason I think we can move those parameters safely.

My previous patch led to issues [4][5] that I'm able to reproduce locally by using oslo.cache's functional test. These changes fix these issues.

[1] https://github.com/sqlalchemy/dogpile.cache/commit/1de93aab14c1274f20c1f44f8adff3b143c864f6
[2] https://github.com/linsomniac/python-memcached/blob/7942465eba2009927e5d14b4b6dbd48b75780d80/memcache.py#L165
[3] https://github.com/sqlalchemy/dogpile.cache/issues/223
[4] https://bugzilla.redhat.com/show_bug.cgi?id=2103117
[5] https://review.opendev.org/c/openstack/requirements/+/848827

Closes: #228
Pull-request: https://github.com/sqlalchemy/dogpile.cache/pull/228
Pull-request-sha: dcef04b200c62d615054b3520ece480825597e61

Change-Id: Ic77c1d657b81449a34114cf9f61c350ffc7e2ba1

3 files changed, 60 insertions, 36 deletions

diff --git a/docs/build/unreleased/224.rst b/docs/build/unreleased/224.rst
new file mode 100644
index 0000000..7f0eaec
--- /dev/null
+++ b/docs/build/unreleased/224.rst
@@ -0,0 +1,11 @@
+.. change::
+    :tags: bug, memcached
+    :tickets: 224
+
+    Moved the :paramref:`.MemcacheArgs.dead_retry` argument and the
+    :paramref:`.MemcacheArgs.socket_timeout` argument which were
+    erroneously added to the "set_parameters",
+    where they have no effect, to be part of the Memcached connection
+    arguments :paramref:`.MemcachedBackend.dead_retry`,
+    :paramref:`.MemcachedBackend.socket_timeout`.
+
diff --git a/dogpile/cache/backends/memcached.py b/dogpile/cache/backends/memcached.py
index 46d5d70..2fe30c7 100644
--- a/dogpile/cache/backends/memcached.py
+++ b/dogpile/cache/backends/memcached.py
@@ -220,16 +220,6 @@ class GenericMemcachedBackend(CacheBackend):
 class MemcacheArgs(GenericMemcachedBackend):
     """Mixin which provides support for the 'time' argument to set(),
     'min_compress_len' to other methods.
-
-    :param dead_retry: Number of seconds memcached server is considered dead
-       before it is tried again..
-
-     .. versionadded:: 1.1.7
-
-    :param socket_timeout: Timeout in seconds for every call to a server.
-
-     .. versionadded:: 1.1.7
-
     """
 
     def __init__(self, arguments):
@@ -242,10 +232,6 @@ class MemcacheArgs(GenericMemcachedBackend):
             self.set_arguments["min_compress_len"] = arguments[
                 "min_compress_len"
             ]
-        if "dead_retry" in arguments:
-            self.set_arguments["dead_retry"] = arguments["dead_retry"]
-        if "socket_timeout" in arguments:
-            self.set_arguments["socket_timeout"] = arguments["socket_timeout"]
         super(MemcacheArgs, self).__init__(arguments)
 
 
@@ -316,14 +302,39 @@ class MemcachedBackend(MemcacheArgs, GenericMemcachedBackend):
             }
         )
 
+    :param dead_retry: Number of seconds memcached server is considered dead
+     before it is tried again. Will be passed to ``memcache.Client``
+     as the ``dead_retry`` parameter.
+
+     .. versionchanged:: 1.1.8  Moved the ``dead_retry`` argument which was
+        erroneously added to "set_parameters" to
+        be part of the Memcached connection arguments.
+
+    :param socket_timeout: Timeout in seconds for every call to a server.
+      Will be passed to ``memcache.Client`` as the ``socket_timeout``
+      parameter.
+
+      .. versionchanged:: 1.1.8  Moved the ``socket_timeout`` argument which
+         was erroneously added to "set_parameters"
+         to be part of the Memcached connection arguments.
+
     """
 
+    def __init__(self, arguments):
+        self.dead_retry = arguments.get("dead_retry", 30)
+        self.socket_timeout = arguments.get("socket_timeout", 3)
+        super(MemcachedBackend, self).__init__(arguments)
+
     def _imports(self):
         global memcache
         import memcache  # noqa
 
     def _create_client(self):
-        return memcache.Client(self.url)
+        return memcache.Client(
+            self.url,
+            dead_retry=self.dead_retry,
+            socket_timeout=self.socket_timeout,
+        )
 
 
 class BMemcachedBackend(GenericMemcachedBackend):
diff --git a/tests/cache/test_memcached_backend.py b/tests/cache/test_memcached_backend.py
index 1d98819..84bfc93 100644
--- a/tests/cache/test_memcached_backend.py
+++ b/tests/cache/test_memcached_backend.py
@@ -380,7 +380,11 @@ class MockMemcacheBackend(MemcachedBackend):
         pass
 
     def _create_client(self):
-        return MockClient(self.url)
+        return MockClient(
+            self.url,
+            dead_retry=self.dead_retry,
+            socket_timeout=self.socket_timeout,
+        )
 
 
 class MockPylibmcBackend(PylibmcBackend):
@@ -422,6 +426,24 @@ class MockClient(object):
         self._cache.pop(key, None)
 
 
+class MemcachedBackendTest(TestCase):
+    def test_memcached_dead_retry(self):
+        config_args = {
+            "url": "127.0.0.1:11211",
+            "dead_retry": 4,
+        }
+        backend = MockMemcacheBackend(arguments=config_args)
+        eq_(backend._create_client().kw["dead_retry"], 4)
+
+    def test_memcached_socket_timeout(self):
+        config_args = {
+            "url": "127.0.0.1:11211",
+            "socket_timeout": 6,
+        }
+        backend = MockMemcacheBackend(arguments=config_args)
+        eq_(backend._create_client().kw["socket_timeout"], 6)
+
+
 class PylibmcArgsTest(TestCase):
     def test_binary_flag(self):
         backend = MockPylibmcBackend(arguments={"url": "foo", "binary": True})
@@ -476,26 +498,6 @@ class MemcachedArgstest(TestCase):
         backend.set("foo", "bar")
         eq_(backend._clients.memcached.canary, [{"min_compress_len": 20}])
 
-    def test_set_dead_retry(self):
-        config_args = {
-            "url": "127.0.0.1:11211",
-            "dead_retry": 4,
-        }
-
-        backend = MockMemcacheBackend(arguments=config_args)
-        backend.set("foo", "bar")
-        eq_(backend._clients.memcached.canary, [{"dead_retry": 4}])
-
-    def test_set_socket_timeout(self):
-        config_args = {
-            "url": "127.0.0.1:11211",
-            "socket_timeout": 4,
-        }
-
-        backend = MockMemcacheBackend(arguments=config_args)
-        backend.set("foo", "bar")
-        eq_(backend._clients.memcached.canary, [{"socket_timeout": 4}])
-
 
 class LocalThreadTest(TestCase):
     def setUp(self):