launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/git-log-better-memcache-key into lp:launchpad.

Commit message:
Remove instance name from git-log memcache key.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-log-better-memcache-key/+merge/295546

Remove instance name from git-log memcache key.  The rest of the key is already a stable identifier, and the instance name tends to be something like "lpnet144" on production so we end up with rather poor memcache utilisation.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-log-better-memcache-key into lp:launchpad.

=== modified file 'lib/lp/code/model/gitref.py'
--- lib/lp/code/model/gitref.py	2016-05-18 11:27:33 +0000
+++ lib/lp/code/model/gitref.py	2016-05-24 06:10:10 +0000
@@ -49,7 +49,6 @@
     BranchMergeProposal,
     BranchMergeProposalGetter,
     )
-from lp.services.config import config
 from lp.services.database.bulk import load_related
 from lp.services.database.constants import UTC_NOW
 from lp.services.database.decoratedresultset import DecoratedResultSet
@@ -260,7 +259,7 @@
         hosting_client = getUtility(IGitHostingClient)
         memcache_client = getUtility(IMemcacheClient)
         path = self.repository.getInternalPath()
-        memcache_key = "%s:git-log:%s:%s" % (config.instance_name, path, start)
+        memcache_key = "git-log:%s:%s" % (path, start)
         if limit is not None:
             memcache_key += ":limit=%s" % limit
         if stop is not None:

=== modified file 'lib/lp/code/model/tests/test_gitref.py'
--- lib/lp/code/model/tests/test_gitref.py	2016-05-18 11:27:33 +0000
+++ lib/lp/code/model/tests/test_gitref.py	2016-05-24 06:10:10 +0000
@@ -27,7 +27,6 @@
 from lp.app.interfaces.informationtype import IInformationType
 from lp.app.interfaces.launchpad import IPrivacy
 from lp.code.interfaces.githosting import IGitHostingClient
-from lp.services.config import config
 from lp.services.features.testing import FeatureFixture
 from lp.services.memcache.interfaces import IMemcacheClient
 from lp.services.webapp.interfaces import OAuthPermission
@@ -168,14 +167,14 @@
                 "commit_message": Equals(u"root"),
                 }),
             ]))
-        key = u"%s:git-log:%s:%s" % (config.instance_name, path, self.sha1_tip)
+        key = u"git-log:%s:%s" % (path, self.sha1_tip)
         self.assertEqual(
             json.dumps(self.log),
             getUtility(IMemcacheClient).get(key.encode("UTF-8")))
 
     def test_cache(self):
         path = self.ref.repository.getInternalPath()
-        key = u"%s:git-log:%s:%s" % (config.instance_name, path, self.sha1_tip)
+        key = u"git-log:%s:%s" % (path, self.sha1_tip)
         getUtility(IMemcacheClient).set(key.encode("UTF-8"), "[]")
         self.assertEqual([], self.ref.getCommits(self.sha1_tip))
 
@@ -191,14 +190,14 @@
             ]))
         self.assertEqual([], self.hosting_client.getLog.calls)
         path = self.ref.repository.getInternalPath()
-        key = u"%s:git-log:%s:%s" % (config.instance_name, path, self.sha1_tip)
+        key = u"git-log:%s:%s" % (path, self.sha1_tip)
         self.assertIsNone(getUtility(IMemcacheClient).get(key.encode("UTF-8")))
 
     def test_disable_memcache(self):
         self.useFixture(
             FeatureFixture({u"code.git.log.disable_memcache": u"on"}))
         path = self.ref.repository.getInternalPath()
-        key = u"%s:git-log:%s:%s" % (config.instance_name, path, self.sha1_tip)
+        key = u"git-log:%s:%s" % (path, self.sha1_tip)
         getUtility(IMemcacheClient).set(key.encode("UTF-8"), "[]")
         self.assertNotEqual([], self.ref.getCommits(self.sha1_tip))
         self.assertEqual(
@@ -211,8 +210,8 @@
             [((path, self.sha1_tip),
               {"limit": 10, "stop": self.sha1_root, "logger": None})],
             self.hosting_client.getLog.calls)
-        key = u"%s:git-log:%s:%s:limit=10:stop=%s" % (
-            config.instance_name, path, self.sha1_tip, self.sha1_root)
+        key = u"git-log:%s:%s:limit=10:stop=%s" % (
+            path, self.sha1_tip, self.sha1_root)
         self.assertEqual(
             json.dumps(self.log),
             getUtility(IMemcacheClient).get(key.encode("UTF-8")))
@@ -224,7 +223,7 @@
         self.assertThat(commits, MatchesListwise([
             ContainsDict({"sha1": Equals(self.sha1_tip)}),
             ]))
-        key = u"%s:git-log:%s:%s" % (config.instance_name, path, self.sha1_tip)
+        key = u"git-log:%s:%s" % (path, self.sha1_tip)
         self.assertEqual(
             json.dumps(self.log),
             getUtility(IMemcacheClient).get(key.encode("UTF-8")))