Merge lp:~jelmer/launchpad/506256-remove-popen into lp:launchpad

[Jelmer Vernooij]

Builddmaster now moves build binaries away for later processing rather than invoking process-uploader on them directly.

buildd-manager currently invokes the uploadprocessor on binaries that it has fetched from the buildd slaves.

As this process is synchronous is blocks the buildd manager from doing other things at the same time - such as scheduling new builds - time during which the buildd slaves are idling.

This branch changes the buildd manager to move build results out of the way into a queue that can independently be processed by process-upload (extensions for process-upload to support this were landed earlier).

Tests:
./bin/test lp.archiveuploader
./bin/test lp.buildmaster

There is still a single test testNoFiles() in the archiveuploader that is itself buggy. I'm looking into this at the moment, but wanted to submit the branch for review earlier because of the upcoming PQM closure. The fix for this test shouldn't involve any changes outside of the test itself.

QA:

This branch has been running on dogfood for the past week or so in its current form and has been working well. We've tested with multiple buildds and thrown several hundred builds at it.

Deployment:

A cron job needs to be set up to run the following command regularly:

LPCURRENT/scripts/process-upload.py -C buildd --builds -v $QUEUE

[Brad Crittenden (bac)]

Jelmer this branch looks ok. As I mentioned on IRC I'm getting test failures. I'm re-running them now and will paste the results when they are done.

Other than that I only find this one typo that needs fixing.

typo: s/nonexisting/nonexistent

Keeping the branch unapproved until the tests are sorted out.

review: Needs Information (code)

[Jelmer Vernooij (jelmer)]

I need to change the lookups in the archiveuploader to be of binarypackagebuilds or sourcepackagebuilds rather than on packagebuilds in general, since otherwise we don't have a sensible verifySuccessBuild() method to call.

[Graham Binns (gmb)]

Abstaining until the tests are fixed.

review: Abstain (release-critical)

[Jelmer Vernooij (jelmer)]

This turned out to be quite a complex issue; I'm fixing the last test now.

[Julian Edwards (julian-edwards)]

My two pence, as I mentioned on Mumble:

 1. Please change the added __getitem__ to a getByID() method. We're deprecating __getitem__
 2. Typo in the new enum (s/process/processed/)

Otherwise, ROCK!

review: Approve

[Brad Crittenden (bac)]

Thanks for fixing the tests Jelmer. They all ran locally for me.

review: Approve (code)

[Graham Binns (gmb)]

RC=me on this. Please land this on db-devel rather than devel (I believe there's some argument you can pass to utils/ec2 in order to achieve this). We want to close devel for landings sooner rather than later, so all traffic should go to db-devel unless absolutely necessary.

review: Approve (release-critical)

=== modified file 'lib/canonical/launchpad/webapp/tales.py'
--- lib/canonical/launchpad/webapp/tales.py	2010-08-27 11:19:54 +0000
+++ lib/canonical/launchpad/webapp/tales.py	2010-09-16 00:48:58 +0000
@@ -995,6 +995,7 @@
             BuildStatus.CHROOTWAIT: {'src': "/@@/build-chrootwait"},
             BuildStatus.SUPERSEDED: {'src': "/@@/build-superseded"},
             BuildStatus.BUILDING: {'src': "/@@/processing"},
+            BuildStatus.UPLOADING: {'src': "/@@/processing"},
             BuildStatus.FAILEDTOUPLOAD: {'src': "/@@/build-failedtoupload"},
             }
 
 
=== modified file 'lib/lp/archiveuploader/tests/test_uploadprocessor.py'
--- lib/lp/archiveuploader/tests/test_uploadprocessor.py	2010-09-02 16:28:50 +0000
+++ lib/lp/archiveuploader/tests/test_uploadprocessor.py	2010-09-16 00:48:58 +0000
@@ -31,6 +31,7 @@
 from canonical.launchpad.testing.fakepackager import FakePackager
 from canonical.launchpad.webapp.errorlog import ErrorReportingUtility
 from canonical.testing import LaunchpadZopelessLayer
+
 from lp.app.errors import NotFoundError
 from lp.archiveuploader.uploadpolicy import (
     AbstractUploadPolicy,
@@ -41,7 +42,10 @@
     parse_build_upload_leaf_name,
     UploadProcessor,
     )
-from lp.buildmaster.enums import BuildStatus
+from lp.buildmaster.enums import (
+    BuildFarmJobType,
+    BuildStatus,
+    )
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.pocket import PackagePublishingPocket
@@ -1861,17 +1865,26 @@
         self.uploadprocessor = self.setupBreezyAndGetUploadProcessor()
 
     def testInvalidLeafName(self):
-        upload_dir = self.queueUpload("bar_1.0-1")
-        self.uploadprocessor.processBuildUpload(upload_dir, "bar_1.0-1")
+        # Directories with invalid leaf names should be skipped,
+        # and a warning logged.
+        upload_dir = self.queueUpload("bar_1.0-1", queue_entry="bar")
+        self.uploadprocessor.processBuildUpload(upload_dir, "bar")
         self.assertLogContains('Unable to extract build id from leaf '
-                               'name bar_1.0-1, skipping.')
+                               'name bar, skipping.')
 
     def testNoBuildEntry(self):
-        upload_dir = self.queueUpload("bar_1.0-1", queue_entry="42-60")
-        self.assertRaises(NotFoundError, self.uploadprocessor.processBuildUpload,
-                upload_dir, "42-60")
+        # Directories with that refer to a nonexistent build
+        # should be skipped and a warning logged.
+        cookie = "%s-%d" % (BuildFarmJobType.PACKAGEBUILD.name, 42)
+        upload_dir = self.queueUpload("bar_1.0-1", queue_entry=cookie)
+        self.uploadprocessor.processBuildUpload(upload_dir, cookie)
+        self.assertLogContains(
+            "Unable to find package build job with id 42. Skipping.")
 
     def testNoFiles(self):
+        # If the upload directory is empty, the upload
+        # will fail.
+
         # Upload a source package
         upload_dir = self.queueUpload("bar_1.0-1")
         self.processUpload(self.uploadprocessor, upload_dir)
@@ -1884,19 +1897,28 @@
             version="1.0-1", name="bar")
         queue_item.setDone()
 
-        build.jobStarted()
-        build.builder = self.factory.makeBuilder()
+        builder = self.factory.makeBuilder()
+        build.buildqueue_record.markAsBuilding(builder)
+        build.builder = build.buildqueue_record.builder
+
+        build.status = BuildStatus.UPLOADING
 
         # Upload and accept a binary for the primary archive source.
         shutil.rmtree(upload_dir)
         self.layer.txn.commit()
-        leaf_name = "%d-%d" % (build.id, 60)
+        leaf_name = build.getUploadDirLeaf(build.getBuildCookie())
         os.mkdir(os.path.join(self.incoming_folder, leaf_name))
         self.options.context = 'buildd'
         self.options.builds = True
-        self.uploadprocessor.processBuildUpload(self.incoming_folder, leaf_name)
+        self.uploadprocessor.processBuildUpload(
+            self.incoming_folder, leaf_name)
+        self.assertEquals(1, len(self.oopses))
         self.layer.txn.commit()
-        self.assertEquals(BuildStatus.FAILEDTOUPLOAD, build.status)
+        self.assertEquals(
+            BuildStatus.FAILEDTOUPLOAD, build.status)
+        self.assertEquals(builder, build.builder)
+        self.assertIsNot(None, build.date_finished)
+        self.assertIsNot(None, build.duration)
         log_contents = build.upload_log.read()
         self.assertTrue('ERROR: Exception while processing upload '
             in log_contents)
@@ -1904,6 +1926,8 @@
             in log_contents)
 
     def testSuccess(self):
+        # Properly uploaded binaries should result in the
+        # build status changing to FULLYBUILT.
         # Upload a source package
         upload_dir = self.queueUpload("bar_1.0-1")
         self.processUpload(self.uploadprocessor, upload_dir)
@@ -1916,21 +1940,27 @@
             version="1.0-1", name="bar")
         queue_item.setDone()
 
-        build.jobStarted()
-        build.builder = self.factory.makeBuilder()
+        build.buildqueue_record.markAsBuilding(self.factory.makeBuilder())
+
+        build.status = BuildStatus.UPLOADING
 
         # Upload and accept a binary for the primary archive source.
         shutil.rmtree(upload_dir)
         self.layer.txn.commit()
-        leaf_name = "%d-%d" % (build.id, 60)
+        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
-        self.uploadprocessor.processBuildUpload(self.incoming_folder, leaf_name)
+        last_stub_mail_count = len(stub.test_emails)
+        self.uploadprocessor.processBuildUpload(
+            self.incoming_folder, leaf_name)
         self.layer.txn.commit()
+        # No emails are sent on success
+        self.assertEquals(len(stub.test_emails), last_stub_mail_count)
         self.assertEquals(BuildStatus.FULLYBUILT, build.status)
-        log_lines = build.upload_log.read().splitlines()
+        log_contents = build.upload_log.read()
+        log_lines = log_contents.splitlines()
         self.assertTrue(
             'INFO: Processing upload bar_1.0-1_i386.changes' in log_lines)
         self.assertTrue(
@@ -1942,10 +1972,9 @@
     """Tests for parse_build_upload_leaf_name."""
 
     def test_valid(self):
-        self.assertEquals((42, 60), parse_build_upload_leaf_name("42-60"))
-
-    def test_invalid_chars(self):
-        self.assertRaises(ValueError, parse_build_upload_leaf_name, "a42-460")
-
-    def test_no_dash(self):
-        self.assertRaises(ValueError, parse_build_upload_leaf_name, "32")
+        self.assertEquals(
+            60, parse_build_upload_leaf_name("20100812-42-PACKAGEBUILD-60"))
+
+    def test_invalid_jobid(self):
+        self.assertRaises(
+            ValueError, parse_build_upload_leaf_name, "aaba-a42-PACKAGEBUILD-abc")
 
=== modified file 'lib/lp/archiveuploader/uploadprocessor.py'
--- lib/lp/archiveuploader/uploadprocessor.py	2010-08-27 11:19:54 +0000
+++ lib/lp/archiveuploader/uploadprocessor.py	2010-09-16 00:48:58 +0000
@@ -74,14 +74,16 @@
     SOURCE_PACKAGE_RECIPE_UPLOAD_POLICY_NAME,
     UploadPolicyError,
     )
-from lp.buildmaster.enums import BuildStatus
+from lp.buildmaster.enums import (
+    BuildStatus,
+    )
+from lp.buildmaster.interfaces.buildfarmjob import IBuildFarmJobSet
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.person import IPersonSet
 from lp.soyuz.interfaces.archive import (
     IArchiveSet,
     NoSuchPPA,
     )
-from lp.soyuz.interfaces.binarypackagebuild import IBinaryPackageBuildSet
 
 
 __all__ = [
@@ -104,11 +106,11 @@
     """Parse the leaf directory name of a build upload.
 
     :param name: Directory name.
-    :return: Tuple with build id and build queue record id.
+    :return: Tuple with build farm job id.
     """
-    (build_id_str, queue_record_str) = name.split("-", 1)
+    (job_id_str,) = name.split("-")[-1:]
     try:
-        return int(build_id_str), int(queue_record_str)
+        return int(job_id_str)
     except TypeError:
         raise ValueError
 
@@ -191,7 +193,7 @@
                     continue
                 if self.builds:
                     # Upload directories contain build results,
-                    # directories are named after build ids.
+                    # directories are named after job ids.
                     self.processBuildUpload(fsroot, upload)
                 else:
                     self.processUpload(fsroot, upload)
@@ -206,13 +208,24 @@
         Build uploads always contain a single package per leaf.
         """
         try:
-            (build_id, build_queue_record_id) = parse_build_upload_leaf_name(
-                upload)
+            job_id = parse_build_upload_leaf_name(upload)
         except ValueError:
             self.log.warn("Unable to extract build id from leaf name %s,"
                 " skipping." % upload)
             return
-        build = getUtility(IBinaryPackageBuildSet).getByBuildID(int(build_id))
+        try:
+            buildfarm_job = getUtility(IBuildFarmJobSet).getByID(job_id)
+        except NotFoundError:
+            self.log.warn(
+                "Unable to find package build job with id %d. Skipping." %
+                job_id)
+            return
+        build = buildfarm_job.getSpecificJob()
+        if build.status != BuildStatus.UPLOADING:
+            self.log.warn(
+                "Expected build status to be 'UPLOADING', was %s. Skipping.",
+                build.status.name)
+            return
         self.log.debug("Build %s found" % build.id)
         logger = BufferLogger()
         upload_path = os.path.join(fsroot, upload)
@@ -246,6 +259,9 @@
             build.notify(extra_info="Uploading build %s failed." % upload)
         build.storeUploadLog(logger.buffer.getvalue())
 
+        # Remove BuildQueue record.
+        build.buildqueue_record.destroySelf()
+
     def processUpload(self, fsroot, upload):
         """Process an upload's changes files, and move it to a new directory.
 
 
=== modified file 'lib/lp/buildmaster/enums.py'
--- lib/lp/buildmaster/enums.py	2010-08-27 15:03:18 +0000
+++ lib/lp/buildmaster/enums.py	2010-09-16 00:48:58 +0000
@@ -97,6 +97,13 @@
         will be notified via process-upload about the reason of the rejection.
         """)
 
+    UPLOADING = DBItem(8, """
+        Uploading build
+
+        The build has completed and is waiting to be processed by the 
+        upload processor.
+        """)
+
 
 class BuildFarmJobType(DBEnumeratedType):
     """Soyuz build farm job type.
@@ -128,6 +135,3 @@
 
         Generate translation templates from a bazaar branch.
         """)
-
-
-
 
=== modified file 'lib/lp/buildmaster/interfaces/buildfarmjob.py'
--- lib/lp/buildmaster/interfaces/buildfarmjob.py	2010-08-30 15:00:23 +0000
+++ lib/lp/buildmaster/interfaces/buildfarmjob.py	2010-09-16 00:48:58 +0000
@@ -292,3 +292,7 @@
             that should be included.
         :return: a `ResultSet` representing the requested builds.
         """
+
+    def getByID(job_id):
+        """Look up a `IBuildFarmJob` record by id.
+        """
 
=== modified file 'lib/lp/buildmaster/interfaces/packagebuild.py'
--- lib/lp/buildmaster/interfaces/packagebuild.py	2010-08-27 11:19:54 +0000
+++ lib/lp/buildmaster/interfaces/packagebuild.py	2010-09-16 00:48:58 +0000
@@ -91,13 +91,6 @@
             title=_("Distribution series"), required=True,
             description=_("Shortcut for its distribution series.")))
 
-    def getUploaderCommand(package_build, upload_leaf, uploader_logfilename):
-        """Get the command to run as the uploader.
-
-        :return: A list of command line arguments, beginning with the
-            executable.
-        """
-
     def getUploadDirLeaf(build_cookie, now=None):
         """Return the directory-leaf where files to be uploaded are stored.
 
@@ -106,24 +99,13 @@
             directory name. If not provided, defaults to now.
         """
 
-    def getUploadDir(upload_leaf):
-        """Return the full directory where files to be uploaded are stored.
-
-        :param upload_leaf: The leaf directory name where things will be
-            stored.
+    def getBuildCookie():
+        """Return the build cookie (build id and build queue record id).
         """
 
     def getLogFromSlave(build):
         """Get last buildlog from slave. """
 
-    def getUploadLogContent(root, leaf):
-        """Retrieve the upload log contents.
-
-        :param root: Root directory for the uploads
-        :param leaf: Leaf for this particular upload
-        :return: Contents of log file or message saying no log file was found.
-        """
-
     def estimateDuration():
         """Estimate the build duration."""
 
 
=== modified file 'lib/lp/buildmaster/model/buildfarmjob.py'
--- lib/lp/buildmaster/model/buildfarmjob.py	2010-08-30 15:00:23 +0000
+++ lib/lp/buildmaster/model/buildfarmjob.py	2010-09-16 00:48:58 +0000
@@ -52,6 +52,7 @@
     IStoreSelector,
     MAIN_STORE,
     )
+from lp.app.errors import NotFoundError
 from lp.buildmaster.enums import BuildStatus
 from lp.buildmaster.enums import BuildFarmJobType
 from lp.buildmaster.interfaces.buildfarmjob import (
@@ -339,6 +340,7 @@
         """See `IBuild`"""
         return self.status not in [BuildStatus.NEEDSBUILD,
                                    BuildStatus.BUILDING,
+                                   BuildStatus.UPLOADING,
                                    BuildStatus.SUPERSEDED]
 
     def getSpecificJob(self):
@@ -431,3 +433,11 @@
         filtered_builds.config(distinct=True)
 
         return filtered_builds
+
+    def getByID(self, job_id):
+        """See `IBuildfarmJobSet`."""
+        job = IStore(BuildFarmJob).find(BuildFarmJob,
+                BuildFarmJob.id == job_id).one()
+        if job is None:
+            raise NotFoundError(job_id)
+        return job
 
=== modified file 'lib/lp/buildmaster/model/packagebuild.py'
--- lib/lp/buildmaster/model/packagebuild.py	2010-09-09 17:02:33 +0000
+++ lib/lp/buildmaster/model/packagebuild.py	2010-09-16 00:48:58 +0000
@@ -14,7 +14,6 @@
 import datetime
 import logging
 import os.path
-import subprocess
 
 from cStringIO import StringIO
 from lazr.delegates import delegates
@@ -36,11 +35,6 @@
 
 from canonical.config import config
 from canonical.database.enumcol import DBEnum
-from canonical.database.sqlbase import (
-    clear_current_connection_cache,
-    cursor,
-    flush_database_updates,
-    )
 from canonical.launchpad.browser.librarian import ProxiedLibraryFileAlias
 from canonical.launchpad.helpers import filenameToContentType
 from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
@@ -73,7 +67,6 @@
 
 
 SLAVE_LOG_FILENAME = 'buildlog'
-UPLOAD_LOG_FILENAME = 'uploader.log'
 
 
 class PackageBuild(BuildFarmJobDerived, Storm):
@@ -164,30 +157,11 @@
         timestamp = now.strftime("%Y%m%d-%H%M%S")
         return '%s-%s' % (timestamp, build_cookie)
 
-    def getUploadDir(self, upload_leaf):
-        """See `IPackageBuild`."""
-        return os.path.join(config.builddmaster.root, 'incoming', upload_leaf)
-
-    @staticmethod
-    def getUploaderCommand(package_build, upload_leaf, upload_logfilename):
-        """See `IPackageBuild`."""
-        root = os.path.abspath(config.builddmaster.root)
-        uploader_command = list(config.builddmaster.uploader.split())
-
-        # Add extra arguments for processing a package upload.
-        extra_args = [
-            "--log-file", "%s" % upload_logfilename,
-            "-d", "%s" % package_build.distribution.name,
-            "-s", "%s" % (
-                package_build.distro_series.getSuite(package_build.pocket)),
-            "-b", "%s" % package_build.id,
-            "-J", "%s" % upload_leaf,
-            '--context=%s' % package_build.policy_name,
-            "%s" % root,
-            ]
-
-        uploader_command.extend(extra_args)
-        return uploader_command
+    def getBuildCookie(self):
+        """See `IPackageBuild`."""
+        return '%s-%s-%s' % (
+            self.id, self.build_farm_job.job_type.name,
+            self.build_farm_job.id)
 
     @staticmethod
     def getLogFromSlave(package_build):
@@ -198,26 +172,6 @@
             package_build.buildqueue_record.getLogFileName(),
             package_build.is_private)
 
-    @staticmethod
-    def getUploadLogContent(root, leaf):
-        """Retrieve the upload log contents.
-
-        :param root: Root directory for the uploads
-        :param leaf: Leaf for this particular upload
-        :return: Contents of log file or message saying no log file was found.
-        """
-        # Retrieve log file content.
-        possible_locations = (
-            'failed', 'failed-to-move', 'rejected', 'accepted')
-        for location_dir in possible_locations:
-            log_filepath = os.path.join(root, location_dir, leaf,
-                UPLOAD_LOG_FILENAME)
-            if os.path.exists(log_filepath):
-                with open(log_filepath, 'r') as uploader_log_file:
-                    return uploader_log_file.read()
-        else:
-            return 'Could not find upload log file'
-
     def estimateDuration(self):
         """See `IPackageBuild`."""
         raise NotImplementedError
@@ -346,19 +300,16 @@
         root = os.path.abspath(config.builddmaster.root)
 
         # Create a single directory to store build result files.
-        upload_leaf = self.getUploadDirLeaf(
-            '%s-%s' % (self.id, self.buildqueue_record.id))
-        upload_dir = self.getUploadDir(upload_leaf)
-        logger.debug("Storing build result at '%s'" % upload_dir)
+        upload_leaf = self.getUploadDirLeaf(self.getBuildCookie())
+        grab_dir = os.path.join(root, "grabbing", upload_leaf)
+        logger.debug("Storing build result at '%s'" % grab_dir)
 
         # Build the right UPLOAD_PATH so the distribution and archive
         # can be correctly found during the upload:
         #       <archive_id>/distribution_name
         # for all destination archive types.
-        archive = self.archive
-        distribution_name = self.distribution.name
-        target_path = '%s/%s' % (archive.id, distribution_name)
-        upload_path = os.path.join(upload_dir, target_path)
+        upload_path = os.path.join(
+            grab_dir, str(self.archive.id), self.distribution.name)
         os.makedirs(upload_path)
 
         slave = removeSecurityProxy(self.buildqueue_record.builder.slave)
@@ -379,106 +330,35 @@
             slave_file = slave.getFile(filemap[filename])
             copy_and_close(slave_file, out_file)
 
+        # Store build information, build record was already updated during
+        # the binary upload.
+        self.storeBuildInfo(self, librarian, slave_status)
+
         # We only attempt the upload if we successfully copied all the
         # files from the slave.
         if successful_copy_from_slave:
-            uploader_logfilename = os.path.join(
-                upload_dir, UPLOAD_LOG_FILENAME)
-            uploader_command = self.getUploaderCommand(
-                self, upload_leaf, uploader_logfilename)
-            logger.debug("Saving uploader log at '%s'" % uploader_logfilename)
-
-            logger.info("Invoking uploader on %s" % root)
-            logger.info("%s" % uploader_command)
-
-            uploader_process = subprocess.Popen(
-                uploader_command, stdout=subprocess.PIPE,
-                stderr=subprocess.PIPE)
-
-            # Nothing should be written to the stdout/stderr.
-            upload_stdout, upload_stderr = uploader_process.communicate()
-
-            # XXX cprov 2007-04-17: we do not check uploader_result_code
-            # anywhere. We need to find out what will be best strategy
-            # when it failed HARD (there is a huge effort in process-upload
-            # to not return error, it only happen when the code is broken).
-            uploader_result_code = uploader_process.returncode
-            logger.info("Uploader returned %d" % uploader_result_code)
-
-        # Quick and dirty hack to carry on on process-upload failures
-        if os.path.exists(upload_dir):
-            logger.warning("The upload directory did not get moved.")
-            failed_dir = os.path.join(root, "failed-to-move")
-            if not os.path.exists(failed_dir):
-                os.mkdir(failed_dir)
-            os.rename(upload_dir, os.path.join(failed_dir, upload_leaf))
-
-        # The famous 'flush_updates + clear_cache' will make visible
-        # the DB changes done in process-upload, considering that the
-        # transaction was set with ISOLATION_LEVEL_READ_COMMITED
-        # isolation level.
-        cur = cursor()
-        cur.execute('SHOW transaction_isolation')
-        isolation_str = cur.fetchone()[0]
-        assert isolation_str == 'read committed', (
-            'BuildMaster/BuilderGroup transaction isolation should be '
-            'ISOLATION_LEVEL_READ_COMMITTED (not "%s")' % isolation_str)
-
-        original_slave = self.buildqueue_record.builder.slave
-
-        # XXX Robert Collins, Celso Providelo 2007-05-26 bug=506256:
-        # 'Refreshing' objects  procedure  is forced on us by using a
-        # different process to do the upload, but as that process runs
-        # in the same unix account, it is simply double handling and we
-        # would be better off to do it within this process.
-        flush_database_updates()
-        clear_current_connection_cache()
-
-        # XXX cprov 2007-06-15: Re-issuing removeSecurityProxy is forced on
-        # us by sqlobject refreshing the builder object during the
-        # transaction cache clearing. Once we sort the previous problem
-        # this step should probably not be required anymore.
-        self.buildqueue_record.builder.setSlaveForTesting(
-            removeSecurityProxy(original_slave))
-
-        # Store build information, build record was already updated during
-        # the binary upload.
-        self.storeBuildInfo(self, librarian, slave_status)
-
-        # Retrive the up-to-date build record and perform consistency
-        # checks. The build record should be updated during the binary
-        # upload processing, if it wasn't something is broken and needs
-        # admins attention. Even when we have a FULLYBUILT build record,
-        # if it is not related with at least one binary, there is also
-        # a problem.
-        # For both situations we will mark the builder as FAILEDTOUPLOAD
-        # and the and update the build details (datebuilt, duration,
-        # buildlog, builder) in LP. A build-failure-notification will be
-        # sent to the lp-build-admin celebrity and to the sourcepackagerelease
-        # uploader about this occurrence. The failure notification will
-        # also contain the information required to manually reprocess the
-        # binary upload when it was the case.
-        if (self.status != BuildStatus.FULLYBUILT or
-            not successful_copy_from_slave or
-            not self.verifySuccessfulUpload()):
-            logger.warning("Build %s upload failed." % self.id)
+            logger.info(
+                "Gathered %s %d completely. Moving %s to uploader queue." % (
+                self.__class__.__name__, self.id, upload_leaf))
+            target_dir = os.path.join(root, "incoming")
+            self.status = BuildStatus.UPLOADING
+        else:
+            logger.warning(
+                "Copy from slave for build %s was unsuccessful.", self.id)
             self.status = BuildStatus.FAILEDTOUPLOAD
-            uploader_log_content = self.getUploadLogContent(root,
-                upload_leaf)
-            # Store the upload_log_contents in librarian so it can be
-            # accessed by anyone with permission to see the build.
-            self.storeUploadLog(uploader_log_content)
-            # Notify the build failure.
-            self.notify(extra_info=uploader_log_content)
-        else:
-            logger.info(
-                "Gathered %s %d completely" % (
-                self.__class__.__name__, self.id))
+            self.notify(extra_info='Copy from slave was unsuccessful.')
+            target_dir = os.path.join(root, "failed")
+
+        if not os.path.exists(target_dir):
+            os.mkdir(target_dir)
+
+        # Move the directory used to grab the binaries into
+        # the incoming directory so the upload processor never
+        # sees half-finished uploads.
+        os.rename(grab_dir, os.path.join(target_dir, upload_leaf))
 
         # Release the builder for another job.
         self.buildqueue_record.builder.cleanSlave()
-        # Remove BuildQueue record.
-        self.buildqueue_record.destroySelf()
 
     def _handleStatus_PACKAGEFAIL(self, librarian, slave_status, logger):
         """Handle a package that had failed to build.
@@ -583,7 +463,9 @@
         unfinished_states = [
             BuildStatus.NEEDSBUILD,
             BuildStatus.BUILDING,
-            BuildStatus.SUPERSEDED]
+            BuildStatus.UPLOADING,
+            BuildStatus.SUPERSEDED,
+            ]
         if status is None or status in unfinished_states:
             result_set.order_by(
                 Desc(BuildFarmJob.date_created), BuildFarmJob.id)
 
=== modified file 'lib/lp/buildmaster/tests/test_buildfarmjob.py'
--- lib/lp/buildmaster/tests/test_buildfarmjob.py	2010-08-30 15:00:23 +0000
+++ lib/lp/buildmaster/tests/test_buildfarmjob.py	2010-09-16 00:48:58 +0000
@@ -22,6 +22,7 @@
     DatabaseFunctionalLayer,
     LaunchpadFunctionalLayer,
     )
+from lp.app.errors import NotFoundError
 from lp.buildmaster.enums import (
     BuildFarmJobType,
     BuildStatus,
@@ -317,3 +318,17 @@
         result = self.build_farm_job_set.getBuildsForBuilder(self.builder)
 
         self.assertEqual([build_1, build_2], list(result))
+
+    def test_getByID(self):
+        # getByID returns a job by id.
+        build_1 = self.makeBuildFarmJob(
+            builder=self.builder,
+            date_finished=datetime(2008, 10, 10, tzinfo=pytz.UTC))
+        flush_database_updates()
+        self.assertEquals(
+            build_1, self.build_farm_job_set.getByID(build_1.id))
+
+    def test_getByID_nonexistant(self):
+        # getByID raises NotFoundError for unknown job ids.
+        self.assertRaises(NotFoundError,
+            self.build_farm_job_set.getByID, 423432432432)
 
=== modified file 'lib/lp/buildmaster/tests/test_packagebuild.py'
--- lib/lp/buildmaster/tests/test_packagebuild.py	2010-09-09 17:02:33 +0000
+++ lib/lp/buildmaster/tests/test_packagebuild.py	2010-09-16 00:48:58 +0000
@@ -9,7 +9,7 @@
 
 from datetime import datetime
 import hashlib
-import os.path
+import os
 
 from storm.store import Store
 from zope.component import getUtility
@@ -22,6 +22,9 @@
     LaunchpadFunctionalLayer,
     LaunchpadZopelessLayer,
     )
+from lp.archiveuploader.uploadprocessor import (
+    parse_build_upload_leaf_name,
+    )
 from lp.buildmaster.enums import (
     BuildFarmJobType,
     BuildStatus,
@@ -34,7 +37,6 @@
 from lp.buildmaster.model.packagebuild import PackageBuild
 from lp.registry.interfaces.pocket import (
     PackagePublishingPocket,
-    pocketsuffix,
     )
 from lp.soyuz.tests.soyuzbuilddhelpers import WaitingSlave
 from lp.testing import (
@@ -192,15 +194,14 @@
             '%s-%s' % (now.strftime("%Y%m%d-%H%M%S"), build_cookie),
             upload_leaf)
 
-    def test_getUploadDir(self):
-        # getUploadDir is the absolute path to the directory in which things
-        # are uploaded to.
-        build_cookie = self.factory.getUniqueInteger()
-        upload_leaf = self.package_build.getUploadDirLeaf(build_cookie)
-        upload_dir = self.package_build.getUploadDir(upload_leaf)
-        self.assertEqual(
-            os.path.join(config.builddmaster.root, 'incoming', upload_leaf),
-            upload_dir)
+    def test_getBuildCookie(self):
+        # A build cookie is made up of the package build id and record id.
+        # The uploadprocessor relies on this format.
+        Store.of(self.package_build).flush()
+        cookie = self.package_build.getBuildCookie()
+        expected_cookie = "%d-PACKAGEBUILD-%d" % (
+            self.package_build.id, self.package_build.build_farm_job.id)
+        self.assertEquals(expected_cookie, cookie)
 
 
 class TestPackageBuildSet(TestPackageBuildBase):
@@ -257,57 +258,18 @@
         super(TestGetUploadMethodsMixin, self).setUp()
         self.build = self.makeBuild()
 
-    def test_getUploadLogContent_nolog(self):
-        """If there is no log file there, a string explanation is returned.
-        """
-        self.useTempDir()
-        self.assertEquals(
-            'Could not find upload log file',
-            self.build.getUploadLogContent(os.getcwd(), "myleaf"))
-
-    def test_getUploadLogContent_only_dir(self):
-        """If there is a directory but no log file, expect the error string,
-        not an exception."""
-        self.useTempDir()
-        os.makedirs("accepted/myleaf")
-        self.assertEquals(
-            'Could not find upload log file',
-            self.build.getUploadLogContent(os.getcwd(), "myleaf"))
-
-    def test_getUploadLogContent_readsfile(self):
-        """If there is a log file, return its contents."""
-        self.useTempDir()
-        os.makedirs("accepted/myleaf")
-        with open('accepted/myleaf/uploader.log', 'w') as f:
-            f.write('foo')
-        self.assertEquals(
-            'foo', self.build.getUploadLogContent(os.getcwd(), "myleaf"))
-
-    def test_getUploaderCommand(self):
-        upload_leaf = self.factory.getUniqueString('upload-leaf')
-        config_args = list(config.builddmaster.uploader.split())
-        log_file = self.factory.getUniqueString('logfile')
-        config_args.extend(
-            ['--log-file', log_file,
-             '-d', self.build.distribution.name,
-             '-s', (self.build.distro_series.name
-                    + pocketsuffix[self.build.pocket]),
-             '-b', str(self.build.id),
-             '-J', upload_leaf,
-             '--context=%s' % self.build.policy_name,
-             os.path.abspath(config.builddmaster.root),
-             ])
-        uploader_command = self.build.getUploaderCommand(
-            self.build, upload_leaf, log_file)
-        self.assertEqual(config_args, uploader_command)
+    def test_getUploadDirLeafCookie_parseable(self):
+        # getUploadDirLeaf should return a directory name
+        # that is parseable by the upload processor.
+        upload_leaf = self.build.getUploadDirLeaf(
+            self.build.getBuildCookie())
+        job_id = parse_build_upload_leaf_name(upload_leaf)
+        self.assertEqual(job_id, self.build.build_farm_job.id)
 
 
 class TestHandleStatusMixin:
     """Tests for `IPackageBuild`s handleStatus method.
 
-    Note: these tests do *not* test the updating of the build
-    status to FULLYBUILT as this happens during the upload which
-    is stubbed out by a mock function.
     """
 
     layer = LaunchpadZopelessLayer
@@ -329,23 +291,23 @@
         builder.setSlaveForTesting(self.slave)
 
         # We overwrite the buildmaster root to use a temp directory.
-        tmp_dir = self.makeTemporaryDirectory()
+        self.upload_root = self.makeTemporaryDirectory()
         tmp_builddmaster_root = """
         [builddmaster]
         root: %s
-        """ % tmp_dir
+        """ % self.upload_root
         config.push('tmp_builddmaster_root', tmp_builddmaster_root)
 
         # We stub out our builds getUploaderCommand() method so
         # we can check whether it was called as well as
         # verifySuccessfulUpload().
-        self.fake_getUploaderCommand = FakeMethod(
-            result=['echo', 'noop'])
-        removeSecurityProxy(self.build).getUploaderCommand = (
-            self.fake_getUploaderCommand)
         removeSecurityProxy(self.build).verifySuccessfulUpload = FakeMethod(
             result=True)
 
+    def assertResultCount(self, count, result):
+        self.assertEquals(
+            1, len(os.listdir(os.path.join(self.upload_root, result))))
+
     def test_handleStatus_OK_normal_file(self):
         # A filemap with plain filenames should not cause a problem.
         # The call to handleStatus will attempt to get the file from
@@ -354,8 +316,8 @@
                 'filemap': {'myfile.py': 'test_file_hash'},
                 })
 
-        self.assertEqual(BuildStatus.FULLYBUILT, self.build.status)
-        self.assertEqual(1, self.fake_getUploaderCommand.call_count)
+        self.assertEqual(BuildStatus.UPLOADING, self.build.status)
+        self.assertResultCount(1, "incoming")
 
     def test_handleStatus_OK_absolute_filepath(self):
         # A filemap that tries to write to files outside of
@@ -364,7 +326,7 @@
             'filemap': {'/tmp/myfile.py': 'test_file_hash'},
             })
         self.assertEqual(BuildStatus.FAILEDTOUPLOAD, self.build.status)
-        self.assertEqual(0, self.fake_getUploaderCommand.call_count)
+        self.assertResultCount(0, "failed")
 
     def test_handleStatus_OK_relative_filepath(self):
         # A filemap that tries to write to files outside of
@@ -373,7 +335,7 @@
             'filemap': {'../myfile.py': 'test_file_hash'},
             })
         self.assertEqual(BuildStatus.FAILEDTOUPLOAD, self.build.status)
-        self.assertEqual(0, self.fake_getUploaderCommand.call_count)
+        self.assertResultCount(0, "failed")
 
     def test_handleStatus_OK_sets_build_log(self):
         # The build log is set during handleStatus.
 
=== modified file 'lib/lp/code/browser/sourcepackagerecipebuild.py'
--- lib/lp/code/browser/sourcepackagerecipebuild.py	2010-08-27 11:19:54 +0000
+++ lib/lp/code/browser/sourcepackagerecipebuild.py	2010-09-16 00:48:58 +0000
@@ -82,6 +82,7 @@
             return 'No suitable builders'
         return {
             BuildStatus.NEEDSBUILD: 'Pending build',
+            BuildStatus.UPLOADING: 'Build uploading',
             BuildStatus.FULLYBUILT: 'Successful build',
             BuildStatus.MANUALDEPWAIT: (
                 'Could not build because of missing dependencies'),
 
=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipebuild.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipebuild.py	2010-09-09 17:02:33 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipebuild.py	2010-09-16 00:48:58 +0000
@@ -354,15 +354,15 @@
             queue_record.builder.setSlaveForTesting(slave)
             return build
 
-        def assertNotifyOnce(status, build):
+        def assertNotifyCount(status, build, count):
             build.handleStatus(status, None, {'filemap': {}})
-            self.assertEqual(1, len(pop_notifications()))
-        for status in ['PACKAGEFAIL', 'OK']:
-            assertNotifyOnce(status, prepare_build())
+            self.assertEqual(count, len(pop_notifications()))
+        assertNotifyCount("PACKAGEFAIL", prepare_build(), 1)
+        assertNotifyCount("OK", prepare_build(), 0)
         build = prepare_build()
         removeSecurityProxy(build).verifySuccessfulUpload = FakeMethod(
-        result=True)
-        assertNotifyOnce('OK', prepare_build())
+                result=True)
+        assertNotifyCount("OK", prepare_build(), 0)
 
 
 class MakeSPRecipeBuildMixin:
 
=== modified file 'lib/lp/registry/model/sourcepackage.py'
--- lib/lp/registry/model/sourcepackage.py	2010-08-27 11:19:54 +0000
+++ lib/lp/registry/model/sourcepackage.py	2010-09-16 00:48:58 +0000
@@ -597,11 +597,15 @@
             % sqlvalues(BuildStatus.FULLYBUILT))
 
         # Ordering according status
-        # * NEEDSBUILD & BUILDING by -lastscore
+        # * NEEDSBUILD, BUILDING & UPLOADING by -lastscore
         # * SUPERSEDED by -datecreated
         # * FULLYBUILT & FAILURES by -datebuilt
         # It should present the builds in a more natural order.
-        if build_state in [BuildStatus.NEEDSBUILD, BuildStatus.BUILDING]:
+        if build_state in [
+            BuildStatus.NEEDSBUILD,
+            BuildStatus.BUILDING,
+            BuildStatus.UPLOADING,
+            ]:
             orderBy = ["-BuildQueue.lastscore"]
             clauseTables.append('BuildPackageJob')
             condition_clauses.append(
 
=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py	2010-08-31 11:31:04 +0000
+++ lib/lp/soyuz/browser/archive.py	2010-09-16 00:48:58 +0000
@@ -947,6 +947,7 @@
             'NEEDSBUILD': 'Waiting to build',
             'FAILEDTOBUILD': 'Failed to build:',
             'BUILDING': 'Currently building',
+            'UPLOADING': 'Currently uploading',
             }
 
         now = datetime.now(tz=pytz.UTC)
 
=== modified file 'lib/lp/soyuz/doc/buildd-slavescanner.txt'
--- lib/lp/soyuz/doc/buildd-slavescanner.txt	2010-08-30 02:07:38 +0000
+++ lib/lp/soyuz/doc/buildd-slavescanner.txt	2010-09-16 00:48:58 +0000
@@ -319,87 +319,27 @@
 This situation happens when the builder has finished the job and is
 waiting for the master to collect its results.
 
-The build record in question can end up in the following states:
-
- * FULLYBUILT: when binaries were collected and uploaded correctly;
- * FAILEDTOUPLOAD: binaries were collected but the upload was
-                   rejected/failed.
-
-
-=== Failed to Upload (FAILEDTOUPLOAD) ===
+The build record in question will end up as UPLOADING.
+
+=== Uploading (UPLOADING) ===
 
     >>> bqItem10 = a_build.queueBuild()
     >>> setupBuildQueue(bqItem10, a_builder)
-    >>> last_stub_mail_count = len(stub.test_emails)
 
 Create a mock slave so the builder gets the right responses for this test.
 
     >>> bqItem10.builder.setSlaveForTesting(
     ...     WaitingSlave('BuildStatus.OK'))
 
-If the build record wasn't updated before/during the updateBuild
-(precisely on binary upload time), the build will be considered
-FAILEDTOUPLOAD:
+The build will progress to the UPLOADING state if the status from
+the builder was OK:
 
     >>> build = getUtility(IBinaryPackageBuildSet).getByQueueEntry(bqItem10)
     >>> a_builder.updateBuild(bqItem10)
-    WARNING:slave-scanner:Build ... upload failed.
-    >>> build.builder is not None
-    True
-    >>> build.date_finished is not None
-    True
-    >>> build.duration is not None
-    True
-    >>> build.log is not None
-    True
-    >>> check_mail_sent(last_stub_mail_count)
-    True
     >>> build.status.title
-    'Failed to upload'
-
-Let's check the emails generated by this 'failure'
-(see build-failedtoupload-workflow.txt for more information):
-
-    >>> from operator import itemgetter
-    >>> local_test_emails = stub.test_emails[last_stub_mail_count:]
-    >>> local_test_emails.sort(key=itemgetter(1), reverse=True)
-    >>> for from_addr, to_addrs, raw_msg in local_test_emails:
-    ...      print to_addrs
-    ['[e-mail]']
-    ['[e-mail]']
-    ['[e-mail]']
-
-Note that a real failed-to-upload notification contains the respective
-upload log information:
-
-    >>> one_email = stub.test_emails.pop()
-    >>> from_addr, to_addrs, raw_msg = one_email
-    >>> print raw_msg
-    Content-Type: text/plain; charset="utf-8"
-    ...
-    X-Launchpad-Build-State: FAILEDTOUPLOAD
-    ...
-    * Build Log: http://.../...i386.mozilla-firefox_0.9_BUILDING.txt.gz
-    ...
-    Upload log:
-    DEBUG   ...
-    DEBUG   Initialising connection.
-    ...
-    DEBUG   Removing lock file: /var/lock/process-upload-buildd.lock
-    ...
-
-When a failure in processing the generated binaries occurs, the log
-output is both emailed in an immediate notification, and stored in the
-librarian for future reference.
-
-    >>> build.upload_log is not None
-    True
-
-What we can clearly notice is that the log is still containing
-the old build state (BUILDING) in its name. This is a minor problem
-that can be sorted by modifying the execution order of procedures
-inside Buildergroup.buildStatus_OK method.
-
+    'Uploading build'
+
+    >>> bqItem10.destroySelf()
 
 === Successfully collected and uploaded  (FULLYBUILT) ===
 
@@ -426,36 +366,14 @@
 
     >>> bqItem10.builder.setSlaveForTesting(WaitingSlave('BuildStatus.OK'))
 
-Now in order to emulate a successfully binary upload we will update
-the build record to FULLYBUILT, as the process-upload would do:
-
-    >>> from lp.buildmaster.enums import BuildStatus
-    >>> build.status = BuildStatus.FULLYBUILT
-
-Now the updateBuild should recognize this build record as a
-Successfully built and uploaded procedure, not sending any
-notification and updating the build information:
-
-    >>> a_builder.updateBuild(bqItem10)
-    >>> build.builder is not None
-    True
-    >>> build.date_finished is not None
-    True
-    >>> build.duration is not None
-    True
-    >>> build.log is not None
-    True
-    >>> build.status.title
-    'Successfully built'
-    >>> check_mail_sent(last_stub_mail_count)
-    False
-
 We do not store any build log information when the binary upload
 processing succeeded.
 
     >>> build.upload_log is None
     True
 
+    >>> bqItem10.destroySelf()
+
 WAITING -> GIVENBACK - slave requested build record to be rescheduled.
 
     >>> bqItem11 = a_build.queueBuild()
@@ -523,6 +441,7 @@
     ...     6).queueBuild()
     >>> setupBuildQueue(bqItem10, a_builder)
     >>> build = bqItem10.specific_job.build
+    >>> from lp.buildmaster.enums import BuildStatus
     >>> build.status = BuildStatus.FULLYBUILT
     >>> bqItem10.builder.setSlaveForTesting(WaitingSlave('BuildStatus.OK'))
 
@@ -613,29 +532,6 @@
     >>> print headers['content-type']
     text/plain
 
-Check the log from the uploader run has made it into the upload directory:
-
-    >>> failed_dir = os.path.join(config.builddmaster.root, 'failed')
-    >>> failed_uploads = sorted(os.listdir(failed_dir))
-    >>> len(failed_uploads)
-    2
-
-    >>> failed_upload = failed_uploads[0]
-    >>> uploader_log = open(os.path.join(failed_dir, failed_upload,
-    ...                                  'uploader.log'))
-
-    >>> print uploader_log.read()
-    DEBUG   ...
-    DEBUG   Initialising connection.
-    DEBUG   Beginning processing
-    DEBUG   Creating directory /var/tmp/builddmaster/accepted
-    DEBUG   Creating directory /var/tmp/builddmaster/rejected
-    DEBUG   Creating directory /var/tmp/builddmaster/failed
-    ...
-    DEBUG   Rolling back any remaining transactions.
-    DEBUG   Removing lock file: /var/lock/process-upload-buildd.lock
-    <BLANKLINE>
-
 Remove build upload results root
 
     >>> shutil.rmtree(config.builddmaster.root)
@@ -1156,7 +1052,6 @@
     >>> build.upload_log = None
     >>> candidate.builder.setSlaveForTesting(WaitingSlave('BuildStatus.OK'))
     >>> a_builder.updateBuild(candidate)
-    WARNING:slave-scanner:Build ... upload failed.
     >>> local_transaction.commit()
 
     >>> build.archive.private
@@ -1167,6 +1062,7 @@
     True
     >>> print lfa.filename
     buildlog_ubuntu-hoary-i386.mozilla-firefox_0.9_BUILDING.txt.gz
+    >>> candidate.destroySelf()
 
 The attempt to fetch the buildlog from the common librarian will fail
 since this is a build in a private archive and the buildlog was thus
 
=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py	2010-08-31 11:31:04 +0000
+++ lib/lp/soyuz/model/archive.py	2010-09-16 00:48:58 +0000
@@ -1004,10 +1004,9 @@
                 BuildStatus.FAILEDTOUPLOAD,
                 BuildStatus.MANUALDEPWAIT,
                 ),
-             # The 'pending' count is a list because we may append to it
-             # later.
             'pending': [
                 BuildStatus.BUILDING,
+                BuildStatus.UPLOADING,
                 ],
             'succeeded': (
                 BuildStatus.FULLYBUILT,
@@ -1023,6 +1022,7 @@
                 BuildStatus.FAILEDTOUPLOAD,
                 BuildStatus.MANUALDEPWAIT,
                 BuildStatus.BUILDING,
+                BuildStatus.UPLOADING,
                 BuildStatus.FULLYBUILT,
                 BuildStatus.SUPERSEDED,
                 ],
@@ -2007,6 +2007,7 @@
                 ),
             'pending': (
                 BuildStatus.BUILDING,
+                BuildStatus.UPLOADING,
                 BuildStatus.NEEDSBUILD,
                 ),
             'succeeded': (
 
=== modified file 'lib/lp/soyuz/model/binarypackagebuild.py'
--- lib/lp/soyuz/model/binarypackagebuild.py	2010-09-09 17:02:33 +0000
+++ lib/lp/soyuz/model/binarypackagebuild.py	2010-09-16 00:48:58 +0000
@@ -251,6 +251,7 @@
         """See `IBuild`"""
         return self.status not in [BuildStatus.NEEDSBUILD,
                                    BuildStatus.BUILDING,
+                                   BuildStatus.UPLOADING,
                                    BuildStatus.SUPERSEDED]
 
     @property
@@ -671,6 +672,10 @@
             buildduration = 'not available'
             buildlog_url = 'not available'
             builder_url = 'not available'
+        elif self.status == BuildStatus.UPLOADING:
+            buildduration = 'uploading'
+            buildlog_url = 'see builder page'
+            builder_url = 'not available'
         elif self.status == BuildStatus.BUILDING:
             # build in process
             buildduration = 'not finished'
@@ -959,11 +964,14 @@
                 % sqlvalues(BuildStatus.FULLYBUILT))
 
         # Ordering according status
-        # * NEEDSBUILD & BUILDING by -lastscore
+        # * NEEDSBUILD, BUILDING & UPLOADING by -lastscore
         # * SUPERSEDED & All by -datecreated
         # * FULLYBUILT & FAILURES by -datebuilt
         # It should present the builds in a more natural order.
-        if status in [BuildStatus.NEEDSBUILD, BuildStatus.BUILDING]:
+        if status in [
+            BuildStatus.NEEDSBUILD,
+            BuildStatus.BUILDING,
+            BuildStatus.UPLOADING]:
             orderBy = ["-BuildQueue.lastscore", "BinaryPackageBuild.id"]
             clauseTables.append('BuildQueue')
             clauseTables.append('BuildPackageJob')
@@ -1079,7 +1087,8 @@
                                 BuildStatus.CHROOTWAIT,
                                 BuildStatus.FAILEDTOUPLOAD)
         needsbuild = collect_builds(BuildStatus.NEEDSBUILD)
-        building = collect_builds(BuildStatus.BUILDING)
+        building = collect_builds(BuildStatus.BUILDING,
+                                  BuildStatus.UPLOADING)
         successful = collect_builds(BuildStatus.FULLYBUILT)
 
         # Note: the BuildStatus DBItems are used here to summarize the