launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-backfill-store-series into lp:launchpad.

Commit message:
Backfill Snap.store_series based on Snap.distro_series.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-backfill-store-series/+merge/300548

Backfill Snap.store_series based on Snap.distro_series.

At the moment, each DistroSeries is only usable with at most one SnappySeries, so we can fill this in unambiguously (although the backfilling code here takes care to check for the case where this is not true).  Doing this is useful because it means we can deal with store/distro series combinations more consistently, and because it reduces the number of steps involved for people to configure existing snap packages to be uploaded to the store.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-backfill-store-series into lp:launchpad.

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2016-07-14 16:07:35 +0000
+++ database/schema/security.cfg	2016-07-20 01:55:19 +0000
@@ -2369,7 +2369,10 @@
 public.previewdiff                      = SELECT, DELETE
 public.revisionauthor                   = SELECT, UPDATE
 public.revisioncache                    = SELECT, DELETE
+public.snap                             = SELECT, UPDATE
 public.snapfile                         = SELECT, DELETE
+public.snappydistroseries               = SELECT
+public.snappyseries                     = SELECT
 public.sourcepackagename                = SELECT
 public.sourcepackagerelease             = SELECT
 public.sourcepackagepublishinghistory   = SELECT, UPDATE

=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py	2016-06-14 14:25:24 +0000
+++ lib/lp/scripts/garbo.py	2016-07-20 01:55:19 +0000
@@ -12,6 +12,7 @@
     'save_garbo_job_state',
     ]
 
+from collections import defaultdict
 from datetime import (
     datetime,
     timedelta,
@@ -119,6 +120,8 @@
 from lp.services.verification.model.logintoken import LoginToken
 from lp.services.webhooks.interfaces import IWebhookJobSource
 from lp.services.webhooks.model import WebhookJob
+from lp.snappy.interfaces.snappyseries import ISnappyDistroSeriesSet
+from lp.snappy.model.snap import Snap
 from lp.soyuz.enums import PackagePublishingStatus
 from lp.soyuz.model.archive import Archive
 from lp.soyuz.model.distributionsourcepackagecache import (
@@ -1553,6 +1556,45 @@
         """
 
 
+class SnapStoreSeriesPopulator(TunableLoop):
+    """Populates Snap.store_series based on Snap.distro_series.
+
+    This only touches rows where there is exactly one SnappySeries that
+    could be built from the relevant DistroSeries.
+    """
+
+    maximum_chunk_size = 5000
+
+    def __init__(self, log, abort_time=None):
+        super(SnapStoreSeriesPopulator, self).__init__(log, abort_time)
+        self.start_at = 1
+        self.store = IMasterStore(Snap)
+        all_series_map = defaultdict(list)
+        for sds in getUtility(ISnappyDistroSeriesSet).getAll():
+            all_series_map[sds.distro_series.id].append(sds.snappy_series)
+        self.series_map = {
+            distro_series_id: snappy_serieses[0]
+            for distro_series_id, snappy_serieses in all_series_map.items()
+            if len(snappy_serieses) == 1}
+
+    def findSnaps(self):
+        return self.store.find(
+            Snap,
+            Snap.id >= self.start_at,
+            Snap.distro_series_id.is_in(self.series_map),
+            Snap.store_series == None).order_by(Snap.id)
+
+    def isDone(self):
+        return self.findSnaps().is_empty()
+
+    def __call__(self, chunk_size):
+        snaps = list(self.findSnaps()[:chunk_size])
+        for snap in snaps:
+            snap.store_series = self.series_map[snap.distro_series_id]
+        self.start_at = snaps[-1].id + 1
+        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.
@@ -1844,6 +1886,7 @@
         RevisionAuthorEmailLinker,
         ScrubPOFileTranslator,
         SnapBuildJobPruner,
+        SnapStoreSeriesPopulator,
         SuggestiveTemplatesCacheUpdater,
         TeamMembershipPruner,
         UnlinkedAccountPruner,

=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py	2016-06-14 12:40:30 +0000
+++ lib/lp/scripts/tests/test_garbo.py	2016-07-20 01:55:19 +0000
@@ -1497,6 +1497,41 @@
         self._test_LiveFSFilePruner(
             'application/octet-stream', 0, expected_count=1)
 
+    def test_SnapStoreSeriesPopulator(self):
+        switch_dbuser('testadmin')
+        # Make some series.
+        dses = [self.factory.makeDistroSeries() for _ in range(4)]
+        sses = [
+            self.factory.makeSnappySeries(usable_distro_series=[dses[1]]),
+            self.factory.makeSnappySeries(usable_distro_series=[dses[2]]),
+            self.factory.makeSnappySeries(usable_distro_series=[dses[3]]),
+            self.factory.makeSnappySeries(usable_distro_series=[dses[3]]),
+            ]
+        # Make some snap packages.
+        snaps = [
+            self.factory.makeSnap(distroseries=dses[0]),
+            self.factory.makeSnap(distroseries=dses[1], store_series=sses[1]),
+            self.factory.makeSnap(distroseries=dses[1]),
+            self.factory.makeSnap(distroseries=dses[2], store_series=sses[0]),
+            self.factory.makeSnap(distroseries=dses[2]),
+            self.factory.makeSnap(distroseries=dses[3]),
+            ]
+        transaction.commit()
+
+        self.runDaily()
+
+        # Snaps with no possible store series are untouched.
+        self.assertIsNone(snaps[0].store_series)
+        # Snaps that already have a store series are untouched.
+        self.assertEqual(sses[1], snaps[1].store_series)
+        self.assertEqual(sses[0], snaps[3].store_series)
+        # Snaps with no current store series and exactly one possible store
+        # series have it filled in.
+        self.assertEqual(sses[0], snaps[2].store_series)
+        self.assertEqual(sses[1], snaps[4].store_series)
+        # Snaps with more than one possible store series are untouched.
+        self.assertIsNone(snaps[5].store_series)
+
 
 class TestGarboTasks(TestCaseWithFactory):
     layer = LaunchpadZopelessLayer