Merge lp:~jelmer/launchpad/uploadprocessor-invalid-status-test into lp:launchpad

[Jelmer Vernooij]

[r=gmb][ui=none][bug=690074] Warn about builds with unexpected status in the archive uploader rather than moving them out of the way.

We've had some issues with the archive uploader moving builds out of its queue too soon. Those issues should be resolved now. This branch makes the archive uploader a bit more cautious and makes it warn (so an OOPS will be logged) about uploads with an unknown build status rather than simply moving their directory out of the way, from which it is much harder to recover.

[Graham Binns (gmb)]

Hi Jelmer,

A couple of styling issues but they're both nitpicks. r=me with these changes.

30 + upload_dir = self.queueUpload("bar_1.0-1_binary",
31 + queue_entry=leaf_name)

Minor styling quibble. Arguments should start on the line after the opening paren.

39 + self.assertTrue(os.path.exists(
40 + os.path.join(self.incoming_folder, leaf_name)))

Same here (you can indent by two levels if necessary).

review: Approve (code)

=== modified file 'lib/lp/archiveuploader/tests/test_uploadprocessor.py'
--- lib/lp/archiveuploader/tests/test_uploadprocessor.py	2010-11-26 16:27:12 +0000
+++ lib/lp/archiveuploader/tests/test_uploadprocessor.py	2010-12-15 22:34:56 +0000
@@ -2033,6 +2033,44 @@
         self.assertIsNot(None, build.duration)
         self.assertIsNot(None, build.upload_log)
 
+    def testBuildWithInvalidStatus(self):
+        # Builds with an invalid (non-UPLOADING) status should trigger
+        # a warning but be left alone.
+        upload_dir = self.queueUpload("bar_1.0-1")
+        self.processUpload(self.uploadprocessor, upload_dir)
+        source_pub = self.publishPackage('bar', '1.0-1')
+        [build] = source_pub.createMissingBuilds()
+
+        # Move the source from the accepted queue.
+        [queue_item] = self.breezy.getQueueItems(
+            status=PackageUploadStatus.ACCEPTED,
+            version="1.0-1", name="bar")
+        queue_item.setDone()
+
+        build.buildqueue_record.markAsBuilding(self.factory.makeBuilder())
+        build.status = BuildStatus.BUILDING
+
+        shutil.rmtree(upload_dir)
+
+        # Commit so the build cookie has the right ids.
+        self.layer.txn.commit()
+        leaf_name = build.getUploadDirLeaf(build.getBuildCookie())
+        upload_dir = self.queueUpload(
+            "bar_1.0-1_binary", queue_entry=leaf_name)
+        self.options.context = 'buildd'
+        self.options.builds = True
+        last_stub_mail_count = len(stub.test_emails)
+        self.uploadprocessor.processBuildUpload(
+            self.incoming_folder, leaf_name)
+        self.layer.txn.commit()
+        # The build status is not changed
+        self.assertTrue(
+            os.path.exists(os.path.join(self.incoming_folder, leaf_name)))
+        self.assertEquals(BuildStatus.BUILDING, build.status)
+        self.assertLogContains(
+            "Expected build status to be 'UPLOADING', was BUILDING. "
+            "Ignoring.")
+
 
 class ParseBuildUploadLeafNameTests(TestCase):
     """Tests for parse_build_upload_leaf_name."""

=== modified file 'lib/lp/archiveuploader/uploadprocessor.py'
--- lib/lp/archiveuploader/uploadprocessor.py	2010-11-24 15:29:49 +0000
+++ lib/lp/archiveuploader/uploadprocessor.py	2010-12-15 22:34:56 +0000
@@ -224,9 +224,8 @@
         build = buildfarm_job.getSpecificJob()
         if build.status != BuildStatus.UPLOADING:
             self.log.warn(
-                "Expected build status to be 'UPLOADING', was %s. "
-                "Moving to failed.", build.status.name)
-            self.moveProcessedUpload(upload_path, "failed", logger)
+                "Expected build status to be 'UPLOADING', was %s. Ignoring." %
+                build.status.name)
             return
         self.log.debug("Build %s found" % build.id)
         try: