← Back to team overview

launchpad-reviewers team mailing list archive

lp:~cjwatson/launchpad/remove-garbo-archivepermission-duplicates into lp:launchpad

 

Colin Watson has proposed merging lp:~cjwatson/launchpad/remove-garbo-archivepermission-duplicates into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/remove-garbo-archivepermission-duplicates/+merge/116251

This garbo job has completed on all instances, so can safely be removed by reverting r15649.
-- 
https://code.launchpad.net/~cjwatson/launchpad/remove-garbo-archivepermission-duplicates/+merge/116251
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/remove-garbo-archivepermission-duplicates into lp:launchpad.
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2012-07-19 04:40:03 +0000
+++ database/schema/security.cfg	2012-07-23 11:31:24 +0000
@@ -2199,7 +2199,6 @@
 [garbo]
 groups=script,read
 public.account                          = SELECT, DELETE
-public.archivepermission                = SELECT, DELETE
 public.answercontact                    = SELECT, DELETE
 public.branch                           = SELECT, UPDATE
 public.branchjob                        = SELECT, DELETE

=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py	2012-07-18 13:47:23 +0000
+++ lib/lp/scripts/garbo.py	2012-07-23 11:31:24 +0000
@@ -93,7 +93,6 @@
     MAIN_STORE,
     MASTER_FLAVOR,
     )
-from lp.soyuz.model.archivepermission import ArchivePermission
 from lp.translations.interfaces.potemplate import IPOTemplateSet
 from lp.translations.model.potmsgset import POTMsgSet
 from lp.translations.model.potranslation import POTranslation
@@ -991,23 +990,6 @@
         transaction.commit()
 
 
-class DuplicateArchivePermissionPruner(BulkPruner):
-    """Cleans up duplicate ArchivePermission rows created by bug 887185."""
-    target_table_class = ArchivePermission
-    ids_to_prune_query = """
-        SELECT id FROM (
-            SELECT id, rank() OVER w AS rank
-            FROM ArchivePermission
-            WHERE packageset IS NOT NULL
-            WINDOW w AS (
-                PARTITION BY person, archive, packageset, permission
-                ORDER BY id
-                )
-            ) AS whatever
-        WHERE rank > 1
-        """
-
-
 class BaseDatabaseGarbageCollector(LaunchpadCronScript):
     """Abstract base class to run a collection of TunableLoops."""
     script_name = None  # Script name for locking and database user. Override.
@@ -1285,7 +1267,6 @@
         BugWatchActivityPruner,
         CodeImportEventPruner,
         CodeImportResultPruner,
-        DuplicateArchivePermissionPruner,
         HWSubmissionEmailLinker,
         LoginTokenPruner,
         ObsoleteBugAttachmentPruner,

=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py	2012-07-18 13:47:23 +0000
+++ lib/lp/scripts/tests/test_garbo.py	2012-07-23 11:31:24 +0000
@@ -97,9 +97,6 @@
     MASTER_FLAVOR,
     )
 from lp.services.worlddata.interfaces.language import ILanguageSet
-from lp.soyuz.enums import ArchivePermissionType
-from lp.soyuz.interfaces.archivepermission import IArchivePermissionSet
-from lp.soyuz.model.archivepermission import ArchivePermission
 from lp.testing import (
     person_logged_in,
     TestCase,
@@ -1019,82 +1016,6 @@
         self.runHourly()
         self.assertNotEqual(old_update, naked_bug.heat_last_updated)
 
-    def test_DuplicateArchivePermissionPruner_removes_duplicate_rows(self):
-        # DuplicateArchivePermissionPruner removes duplicated packageset
-        # permissions.
-        switch_dbuser('testadmin')
-        archive = self.factory.makeArchive()
-        person = self.factory.makePerson()
-        packageset = self.factory.makePackageset()
-        for _ in range(3):
-            ArchivePermission(
-                archive=archive, person=person, packageset=packageset,
-                permission=ArchivePermissionType.UPLOAD)
-        ap_set = getUtility(IArchivePermissionSet)
-        self.assertEqual(
-            3, ap_set.uploadersForPackageset(archive, packageset).count())
-        self.runDaily()
-        self.assertEqual(
-            1, ap_set.uploadersForPackageset(archive, packageset).count())
-
-    def test_DuplicateArchivePermissionPruner_skips_unique_rows(self):
-        # DuplicateArchivePermissionPruner leaves unique packageset
-        # permissions alone.
-        switch_dbuser('testadmin')
-        archive_one = self.factory.makeArchive()
-        archive_two = self.factory.makeArchive()
-        person_one = self.factory.makePerson()
-        person_two = self.factory.makePerson()
-        packageset_one = self.factory.makePackageset()
-        packageset_two = self.factory.makePackageset()
-        for archive, person, packageset in (
-            (archive_one, person_one, packageset_one),
-            (archive_two, person_one, packageset_one),
-            (archive_one, person_two, packageset_one),
-            (archive_one, person_one, packageset_two),
-            ):
-            ArchivePermission(
-                archive=archive, person=person, packageset=packageset,
-                permission=ArchivePermissionType.UPLOAD)
-        ap_set = getUtility(IArchivePermissionSet)
-        self.assertEqual(
-            2,
-            ap_set.uploadersForPackageset(archive_one, packageset_one).count())
-        self.assertEqual(
-            1,
-            ap_set.uploadersForPackageset(archive_two, packageset_one).count())
-        self.assertEqual(
-            1,
-            ap_set.uploadersForPackageset(archive_one, packageset_two).count())
-        self.runDaily()
-        self.assertEqual(
-            2,
-            ap_set.uploadersForPackageset(archive_one, packageset_one).count())
-        self.assertEqual(
-            1,
-            ap_set.uploadersForPackageset(archive_two, packageset_one).count())
-        self.assertEqual(
-            1,
-            ap_set.uploadersForPackageset(archive_one, packageset_two).count())
-
-    def test_DuplicateArchivePermissionPruner_skips_non_packagesets(self):
-        # DuplicateArchivePermissionPruner leaves non-packageset permissions
-        # alone.
-        switch_dbuser('testadmin')
-        archive = self.factory.makeArchive()
-        person = self.factory.makePerson()
-        component = self.factory.makeComponent()
-        for _ in range(3):
-            ArchivePermission(
-                archive=archive, person=person, component=component,
-                permission=ArchivePermissionType.UPLOAD)
-        ap_set = getUtility(IArchivePermissionSet)
-        self.assertEqual(
-            3, ap_set.uploadersForComponent(archive, component).count())
-        self.runDaily()
-        self.assertEqual(
-            3, ap_set.uploadersForComponent(archive, component).count())
-
 
 class TestGarboTasks(TestCaseWithFactory):
     layer = LaunchpadZopelessLayer


Follow ups