launchpad-reviewers team mailing list archive

[Ioana Lasc <mp+407679@xxxxxxxxxxxxxxxxxx>]

Ioana Lasc has proposed merging ~ilasc/launchpad:populate-store-upload-revision into launchpad:master.

Commit message:
Populate store_upload_revision

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

This populates the new DB column store_upload_revision on SnapBuild. 
We're renaming the existent "store_upload_revision" property on SnapBuild to "store_upload_revision_property". We will be using this property to populate the new DB column "store_upload_revision" in PopulateSnapBuildStoreRevision.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ilasc/launchpad:populate-store-upload-revision into launchpad:master.

diff --git a/lib/lp/scripts/garbo.py b/lib/lp/scripts/garbo.py
index fba6c29..d781951 100644
--- a/lib/lp/scripts/garbo.py
+++ b/lib/lp/scripts/garbo.py
@@ -37,6 +37,7 @@ from storm.expr import (
     Except,
     In,
     Join,
+    LeftJoin,
     Max,
     Min,
     Or,
@@ -134,8 +135,14 @@ from lp.services.verification.model.logintoken import LoginToken
 from lp.services.webapp.publisher import canonical_url
 from lp.services.webhooks.interfaces import IWebhookJobSource
 from lp.services.webhooks.model import WebhookJob
-from lp.snappy.model.snapbuild import SnapFile
-from lp.snappy.model.snapbuildjob import SnapBuildJobType
+from lp.snappy.model.snapbuild import (
+    SnapBuild,
+    SnapFile,
+    )
+from lp.snappy.model.snapbuildjob import (
+    SnapBuildJob,
+    SnapBuildJobType,
+    )
 from lp.soyuz.enums import ArchiveSubscriberStatus
 from lp.soyuz.interfaces.publishing import active_publishing_status
 from lp.soyuz.model.archive import Archive
@@ -1717,6 +1724,47 @@ class ArchiveAuthTokenDeactivator(BulkPruner):
         transaction.commit()
 
 
+class PopulateSnapBuildStoreRevision(TunableLoop):
+    """Populates snapbuild.store_upload_revision if not set."""
+
+    maximum_chunk_size = 5000
+
+    def __init__(self, log, abort_time=None):
+        super(PopulateSnapBuildStoreRevision, self).__init__(log, abort_time)
+        self.start_at = 1
+        self.store = IMasterStore(SnapBuild)
+
+    def findSnapBuilds(self):
+        origin = [
+            SnapBuild,
+            LeftJoin(
+                SnapBuildJob,
+                SnapBuildJob.snapbuild_id == SnapBuild.id),
+            LeftJoin(
+                Job,
+                Job.id == SnapBuildJob.job_id)
+            ]
+        builds = self.store.using(*origin).find(
+            (SnapBuild),
+            SnapBuild.id >= self.start_at,
+            SnapBuild.store_upload_revision == None,
+            SnapBuildJob.job_type == SnapBuildJobType.STORE_UPLOAD,
+            Job._status == JobStatus.COMPLETED)
+
+        return builds.order_by(SnapBuild.id)
+
+    def isDone(self):
+        return self.findSnapBuilds().is_empty()
+
+    def __call__(self, chunk_size):
+        builds = list(self.findSnapBuilds()[:chunk_size])
+        for build in builds:
+            build.store_upload_revision = build.store_upload_revision_property
+        if len(builds):
+            self.start_at = builds[-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.
@@ -2008,6 +2056,7 @@ class DailyDatabaseGarbageCollector(BaseDatabaseGarbageCollector):
         OCIFilePruner,
         ObsoleteBugAttachmentPruner,
         OldTimeLimitedTokenDeleter,
+        PopulateSnapBuildStoreRevision,
         POTranslationPruner,
         PreviewDiffPruner,
         ProductVCSPopulator,
diff --git a/lib/lp/scripts/tests/test_garbo.py b/lib/lp/scripts/tests/test_garbo.py
index 0d34fce..6e685b5 100644
--- a/lib/lp/scripts/tests/test_garbo.py
+++ b/lib/lp/scripts/tests/test_garbo.py
@@ -97,6 +97,7 @@ from lp.scripts.garbo import (
     load_garbo_job_state,
     LoginTokenPruner,
     OpenIDConsumerAssociationPruner,
+    PopulateSnapBuildStoreRevision,
     ProductVCSPopulator,
     save_garbo_job_state,
     UnusedPOTMsgSetPruner,
@@ -130,11 +131,17 @@ from lp.services.verification.interfaces.authtoken import LoginTokenType
 from lp.services.verification.model.logintoken import LoginToken
 from lp.services.worlddata.interfaces.language import ILanguageSet
 from lp.snappy.interfaces.snap import SNAP_TESTING_FLAGS
+from lp.snappy.interfaces.snapbuildjob import ISnapStoreUploadJobSource
+from lp.snappy.interfaces.snapstoreclient import ISnapStoreClient
 from lp.snappy.model.snapbuild import SnapFile
 from lp.snappy.model.snapbuildjob import (
     SnapBuildJob,
     SnapStoreUploadJob,
     )
+from lp.snappy.tests.test_snapbuildjob import (
+    FakeSnapStoreClient,
+    run_isolated_jobs,
+    )
 from lp.soyuz.enums import (
     ArchiveSubscriberStatus,
     PackagePublishingStatus,
@@ -153,7 +160,11 @@ from lp.testing import (
     TestCase,
     TestCaseWithFactory,
     )
-from lp.testing.dbuser import switch_dbuser
+from lp.testing.dbuser import (
+    dbuser,
+    switch_dbuser,
+    )
+from lp.testing.fixture import ZopeUtilityFixture
 from lp.testing.layers import (
     DatabaseLayer,
     LaunchpadScriptLayer,
@@ -1979,6 +1990,79 @@ class TestGarbo(FakeAdapterMixin, TestCaseWithFactory):
         self.assertEmailQueueLength(0)
 
 
+    def test_PopulateSnapBuildStoreRevision_triggered_correctly(self):
+        switch_dbuser('testadmin')
+        snap1 = self.factory.makeSnap()
+        self.factory.makeSnapBuild(
+            snap=snap1,
+            status=BuildStatus.FULLYBUILT)
+
+        # This should trigger PopulateSnapBuildStoreRevision but doesn't
+        self.runDaily()
+
+    def test_PopulateSnapBuildStoreRevision(self):
+        switch_dbuser('testadmin')
+        snap1 = self.factory.makeSnap()
+        build1 = self.factory.makeSnapBuild(
+            snap=snap1,
+            status=BuildStatus.FULLYBUILT)
+
+        # test that build1 does not get picked up
+        # as it is a build without a store upload
+        populator = PopulateSnapBuildStoreRevision(None)
+        rs = populator.findSnapBuilds()
+        self.assertEqual(0, rs.count())
+
+        # Upload build and test it finds this one build that has no
+        # store_upload_revision set yet
+        job = getUtility(ISnapStoreUploadJobSource).create(build1)
+        client = FakeSnapStoreClient()
+        client.upload.result = (
+            "http://sca.example/dev/api/snaps/1/builds/1/status";)
+        client.checkStatus.result = (
+            "http://sca.example/dev/click-apps/1/rev/1/";, 1)
+        self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
+        with dbuser(config.ISnapStoreUploadJobSource.dbuser):
+            run_isolated_jobs([job])
+
+        populator = PopulateSnapBuildStoreRevision(None)
+        filter = populator.findSnapBuilds()
+        self.assertEqual(1, filter.count())
+        self.assertEqual(build1, filter.one())
+
+        populator.__call__(5)
+        self.assertEqual(build1.store_upload_revision, 1)
+
+        # Tests that of all builds for the same snap only those that have
+        # been uploaded to the store will get
+        # their new store_upload_revision DB field updated
+        build2 = self.factory.makeSnapBuild(
+            snap=snap1,
+            status=BuildStatus.FULLYBUILT)
+        build3 = self.factory.makeSnapBuild(
+            snap=snap1,
+            status=BuildStatus.FULLYBUILT)
+        job = getUtility(ISnapStoreUploadJobSource).create(build2)
+        client = FakeSnapStoreClient()
+        client.upload.result = (
+            "http://sca.example/dev/api/snaps/1/builds/2/status";)
+        client.checkStatus.result = (
+            "http://sca.example/dev/click-apps/1/rev/1/";, 1)
+        self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
+        with dbuser(config.ISnapStoreUploadJobSource.dbuser):
+            run_isolated_jobs([job])
+
+        populator = PopulateSnapBuildStoreRevision(None)
+
+        filter = populator.findSnapBuilds()
+        self.assertEqual(1, filter.count())
+        self.assertEqual(build2, filter.one())
+
+        populator.__call__(5)
+        self.assertEqual(build2.store_upload_revision, 1)
+        self.assertIsNone(build3.store_upload_revision)
+
+
 class TestGarboTasks(TestCaseWithFactory):
     layer = LaunchpadZopelessLayer
 
diff --git a/lib/lp/snappy/model/snapbuild.py b/lib/lp/snappy/model/snapbuild.py
index ca5285a..909d407 100644
--- a/lib/lp/snappy/model/snapbuild.py
+++ b/lib/lp/snappy/model/snapbuild.py
@@ -191,12 +191,15 @@ class SnapBuild(PackageBuildMixin, Storm):
 
     failure_count = Int(name='failure_count', allow_none=False)
 
+    store_upload_revision = Int(name='store_upload_revision', allow_none=True)
+
     store_upload_metadata = JSON('store_upload_json_data', allow_none=True)
 
     def __init__(self, build_farm_job, requester, snap, archive,
                  distro_arch_series, pocket, snap_base, channels,
                  processor, virtualized, date_created,
-                 store_upload_metadata=None, build_request=None):
+                 store_upload_metadata=None, store_upload_revision=None,
+                 build_request=None):
         """Construct a `SnapBuild`."""
         super(SnapBuild, self).__init__()
         self.build_farm_job = build_farm_job
@@ -211,6 +214,7 @@ class SnapBuild(PackageBuildMixin, Storm):
         self.virtualized = virtualized
         self.date_created = date_created
         self.store_upload_metadata = store_upload_metadata
+        self.store_upload_revision = store_upload_revision
         if build_request is not None:
             self.build_request_id = build_request.id
         self.status = BuildStatus.NEEDSBUILD
@@ -526,7 +530,11 @@ class SnapBuild(PackageBuildMixin, Storm):
         return job and job.store_url
 
     @property
-    def store_upload_revision(self):
+    def store_upload_revision_property(self):
+        # We're renaming this from "store_upload_revision"
+        # as we have added a DB column called "store_upload_revision".
+        # We will be using this property to populate the new
+        # DB column "store_upload_revision" in PopulateSnapBuildStoreRevision
         job = self.last_store_upload_job
         return job and job.store_revision