launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/garbo-up-legacy-access into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #956707 in Launchpad itself: "No way to migrate existing bugs to the new access model"
  https://bugs.launchpad.net/launchpad/+bug/956707

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/garbo-up-legacy-access/+merge/97798

This branch adds a one-off garbo job to mirror existing bugs to the new access model. There are triggers to cope with changes, but previously nothing to migrate existing untouched data.

Since it's difficult and slow to check if a bug has already been mirrored, and this only needs to be done once for each bug, I make a single pass over the bugs by ID, storing a watermark in memcached after each batch. It's not problematic if it gets forgotten, just a bit of a performance hit.
-- 
https://code.launchpad.net/~wgrant/launchpad/garbo-up-legacy-access/+merge/97798
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/garbo-up-legacy-access into lp:launchpad.

=== modified file 'lib/lp/registry/model/accesspolicy.py'
--- lib/lp/registry/model/accesspolicy.py	2012-03-15 08:54:40 +0000
+++ lib/lp/registry/model/accesspolicy.py	2012-03-16 05:52:21 +0000
@@ -32,6 +32,7 @@
     IAccessArtifactGrantSource,
     IAccessPolicy,
     IAccessPolicyArtifact,
+    IAccessPolicyArtifactSource,
     IAccessPolicyGrant,
     )
 from lp.registry.model.person import Person
@@ -113,6 +114,7 @@
             return
         ids = [abstract.id for abstract in abstracts]
         getUtility(IAccessArtifactGrantSource).revokeByArtifact(abstracts)
+        getUtility(IAccessPolicyArtifactSource).deleteByArtifact(abstracts)
         IStore(abstract).find(cls, cls.id.is_in(ids)).remove()
 
 

=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py	2012-03-16 00:43:22 +0000
+++ lib/lp/scripts/garbo.py	2012-03-16 05:52:21 +0000
@@ -81,6 +81,7 @@
 from lp.services.librarian.model import TimeLimitedToken
 from lp.services.log.logger import PrefixFilter
 from lp.services.looptuner import TunableLoop
+from lp.services.memcache.interfaces import IMemcacheClient
 from lp.services.oauth.model import OAuthNonce
 from lp.services.openid.model.openidconsumer import OpenIDConsumerNonce
 from lp.services.propertycache import cachedproperty
@@ -1113,6 +1114,40 @@
         self.transaction.commit()
 
 
+class BugLegacyAccessMirrorer(TunableLoop):
+    """A `TunableLoop` to populate the access policy schema for all bugs."""
+
+    maximum_chunk_size = 5000
+
+    def __init__(self, log, abort_time=None):
+        super(BugLegacyAccessMirrorer, self).__init__(log, abort_time)
+        watermark = getUtility(IMemcacheClient).get(
+            '%s:bug-legacy-access-mirrorer' % config.instance_name)
+        self.start_at = watermark or 0
+
+    def findBugIDs(self):
+        return IMasterStore(Bug).find(
+            (Bug.id,), Bug.id >= self.start_at).order_by(Bug.id)
+
+    def isDone(self):
+        return self.findBugIDs().is_empty()
+
+    def __call__(self, chunk_size):
+        ids = [row[0] for row in self.findBugIDs()[:chunk_size]]
+        list(IMasterStore(Bug).using(Bug).find(
+            SQL('bug_mirror_legacy_access(Bug.id)'),
+            Bug.id.is_in(ids)))
+
+        self.start_at = ids[-1] + 1
+        result = getUtility(IMemcacheClient).set(
+            '%s:bug-legacy-access-mirrorer' % config.instance_name,
+            self.start_at)
+        if not result:
+            self.log.warning('Failed to set start_at in memcache.')
+
+        transaction.commit()
+
+
 class BaseDatabaseGarbageCollector(LaunchpadCronScript):
     """Abstract base class to run a collection of TunableLoops."""
     script_name = None  # Script name for locking and database user. Override.
@@ -1365,6 +1400,7 @@
         DuplicateSessionPruner,
         BugHeatUpdater,
         BugsInformationTypeMigrator,
+        BugLegacyAccessMirrorer,
         ]
     experimental_tunable_loops = []
 

=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py	2012-03-16 00:43:22 +0000
+++ lib/lp/scripts/tests/test_garbo.py	2012-03-16 05:52:21 +0000
@@ -55,6 +55,7 @@
 from lp.code.model.codeimportevent import CodeImportEvent
 from lp.code.model.codeimportresult import CodeImportResult
 from lp.registry.enums import InformationType
+from lp.registry.interfaces.accesspolicy import IAccessArtifactSource
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.person import IPersonSet
 from lp.scripts.garbo import (
@@ -1115,6 +1116,26 @@
         self.runHourly()
         self.assertEqual(InformationType.USERDATA, bug.information_type)
 
+    def test_BugLegacyAccessMirrorer(self):
+        # Private bugs without corresponding data in the access policy
+        # schema get mirrored.
+        switch_dbuser('testadmin')
+        bug = self.factory.makeBug(private=True)
+        # Remove the existing mirrored data.
+        getUtility(IAccessArtifactSource).delete([bug])
+        transaction.commit()
+        self.runHourly()
+        # Check that there's an artifact again, and delete it.
+        switch_dbuser('testadmin')
+        [artifact] = getUtility(IAccessArtifactSource).find([bug])
+        getUtility(IAccessArtifactSource).delete([bug])
+        transaction.commit()
+        self.runHourly()
+        # A watermark is kept in memcache, so a second run doesn't
+        # consider the same bug.
+        self.assertContentEqual(
+            [], getUtility(IAccessArtifactSource).find([bug]))
+
 
 class TestGarboTasks(TestCaseWithFactory):
     layer = LaunchpadZopelessLayer