launchpad-reviewers team mailing list archive

[Jürgen Gmach <mp+411523@xxxxxxxxxxxxxxxxxx>]

Jürgen Gmach has proposed merging ~jugmac00/launchpad:replace-python-memcached-with-pymemcache into launchpad:master.

Commit message:
Replace unmaintained python-memcached with pymemcache

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jugmac00/launchpad/+git/launchpad/+merge/411523

This is work in progress.

Trigger the "Connection refused" issue with running

bin/test -vcct test_memcache_client
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~jugmac00/launchpad:replace-python-memcached-with-pymemcache into launchpad:master.

diff --git a/lib/lp/services/memcache/client.py b/lib/lp/services/memcache/client.py
index a672c99..5ee056a 100644
--- a/lib/lp/services/memcache/client.py
+++ b/lib/lp/services/memcache/client.py
@@ -9,21 +9,34 @@ __all__ = [
 
 import re
 
-import memcache
+from pymemcache.client.base import Client
+from pymemcache.serde import (
+    python_memcache_deserializer,
+    python_memcache_serializer,
+    )
 
 from lp.services.config import config
 
 
 def memcache_client_factory(timeline=True):
-    """Return a memcache.Client for Launchpad."""
+    """Return a pymemcache Client for Launchpad."""
     servers = [
         (host, int(weight)) for host, weight in re.findall(
             r'\((.+?),(\d+)\)', config.memcache.servers)]
     assert len(servers) > 0, "Invalid memcached server list %r" % (
         config.memcache.servers,)
+
+    # pymemcache.client.base.Client only takes a single server
+    # servers is something like [('127.0.0.1:11242', 1)]
+    server = servers[0][0]
+
     if timeline:
         from lp.services.memcache.timeline import TimelineRecordingClient
         client_factory = TimelineRecordingClient
     else:
-        client_factory = memcache.Client
-    return client_factory(servers)
+        client_factory = Client
+    return client_factory(
+        server,
+        serializer=python_memcache_serializer,
+        deserializer=python_memcache_deserializer
+    )
diff --git a/lib/lp/services/memcache/tests/test_memcache_client.py b/lib/lp/services/memcache/tests/test_memcache_client.py
index 46840f0..2166060 100644
--- a/lib/lp/services/memcache/tests/test_memcache_client.py
+++ b/lib/lp/services/memcache/tests/test_memcache_client.py
@@ -4,6 +4,7 @@
 """Tests for the IMemcacheClient utility."""
 
 from lazr.restful.utils import get_current_browser_request
+from pymemcache.exceptions import MemcacheIllegalInputError
 from zope.component import getUtility
 
 from lp.services.memcache.client import memcache_client_factory
@@ -33,7 +34,7 @@ class MemcacheClientTestCase(TestCase):
         we upgrade we upgrade to a version with correct validation.
         """
         self.assertRaises(
-            self.client.MemcachedKeyCharacterError,
+            MemcacheIllegalInputError,
             self.client.set, 'key with spaces', 'some value')
 
     def test_set_recorded_to_timeline(self):
diff --git a/lib/lp/services/memcache/timeline.py b/lib/lp/services/memcache/timeline.py
index 0e898f8..1ec4d2a 100644
--- a/lib/lp/services/memcache/timeline.py
+++ b/lib/lp/services/memcache/timeline.py
@@ -10,13 +10,13 @@ __all__ = [
 import logging
 
 from lazr.restful.utils import get_current_browser_request
-import memcache
+from pymemcache.client.base import Client
 
 from lp.services import features
 from lp.services.timeline.requesttimeline import get_request_timeline
 
 
-class TimelineRecordingClient(memcache.Client):
+class TimelineRecordingClient(Client):
 
     def __get_timeline_action(self, suffix, key):
         request = get_current_browser_request()
@@ -36,17 +36,16 @@ class TimelineRecordingClient(memcache.Client):
             return None
         action = self.__get_timeline_action("get", key)
         try:
-            return memcache.Client.get(self, key)
+            return Client.get(self, key)
         finally:
             action.finish()
 
-    def set(self, key, value, time=0, min_compress_len=0):
+    def set(self, key, value, time=0):
         if not self._enabled:
             return None
         action = self.__get_timeline_action("set", key)
         try:
-            success = memcache.Client.set(self, key, value, time=time,
-                min_compress_len=min_compress_len)
+            success = Client.set(self, key, value, expire=time)
             if success:
                 logging.debug("Memcache set succeeded for %s", key)
             else:
diff --git a/lib/lp/testing/layers.py b/lib/lp/testing/layers.py
index 632a255..1503410 100644
--- a/lib/lp/testing/layers.py
+++ b/lib/lp/testing/layers.py
@@ -543,8 +543,11 @@ class MemcachedLayer(BaseLayer):
         # First, check to see if there is a memcached already running.
         # This happens when new layers are run as a subprocess.
         test_key = "MemcachedLayer__live_test"
-        if MemcachedLayer.client.set(test_key, "live"):
-            return
+        try:
+            if MemcachedLayer.client.set(test_key, "live"):
+                return
+        except OSError:
+            pass
 
         # memcached >= 1.4.29 requires the item size to be at most a quarter
         # of the memory size; 1.5.4 lifts this restriction to at most half
@@ -574,13 +577,16 @@ class MemcachedLayer(BaseLayer):
         MemcachedLayer._memcached_process.stdin.close()
 
         # Wait for the memcached to become operational.
-        while not MemcachedLayer.client.set(test_key, "live"):
-            if MemcachedLayer._memcached_process.returncode is not None:
-                raise LayerInvariantError(
-                    "memcached never started or has died.",
-                    MemcachedLayer._memcached_process.stdout.read())
-            MemcachedLayer.client.forget_dead_hosts()
-            time.sleep(0.1)
+        while True:
+            try:
+                if MemcachedLayer.client.set(test_key, "live"):
+                    break
+            except OSError:
+                if MemcachedLayer._memcached_process.returncode is not None:
+                    raise LayerInvariantError(
+                        "memcached never started or has died.",
+                        MemcachedLayer._memcached_process.stdout.read())
+                time.sleep(0.1)
 
         # Store the pidfile for other processes to kill.
         pid_file = MemcachedLayer.getPidFile()
@@ -612,7 +618,6 @@ class MemcachedLayer(BaseLayer):
     @classmethod
     @profiled
     def testSetUp(cls):
-        MemcachedLayer.client.forget_dead_hosts()
         MemcachedLayer.client.flush_all()
 
     @classmethod
diff --git a/lib/lp/testing/tests/test_layers_functional.py b/lib/lp/testing/tests/test_layers_functional.py
index 1802061..9796fb5 100644
--- a/lib/lp/testing/tests/test_layers_functional.py
+++ b/lib/lp/testing/tests/test_layers_functional.py
@@ -253,7 +253,6 @@ class BaseTestCase(TestCase):
         # reports memcached did not die.(self):
         client = MemcachedLayer.client or memcache_client_factory()
         key = "BaseTestCase.testMemcachedWorking"
-        client.forget_dead_hosts()
         is_live = client.set(key, "live")
         if self.want_memcached:
             self.assertEqual(
diff --git a/requirements/launchpad.txt b/requirements/launchpad.txt
index 0e6ff1e..9da8e98 100644
--- a/requirements/launchpad.txt
+++ b/requirements/launchpad.txt
@@ -119,6 +119,7 @@ pygpgme==0.3+lp1
 PyHamcrest==1.9.0
 pyinotify==0.9.4
 pymacaroons==0.13.0
+pymemcache==3.5.0
 PyNaCl==1.3.0
 pyOpenSSL==17.5.0
 pystache==0.5.3
diff --git a/setup.cfg b/setup.cfg
index 70efbe9..da40348 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -70,6 +70,7 @@ install_requires =
     pygettextpo
     pygpgme
     pymacaroons
+    pymemcache
     pystache
     python-debian
     python-keystoneclient