launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/check-job-status-on-accept into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #941926 in Launchpad itself: "InvalidTransition: Transition from Completed to Waiting is invalid."
  https://bugs.launchpad.net/launchpad/+bug/941926

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/check-job-status-on-accept/+merge/173396

Check the job when accepting it from the queue and raise an error if the job has already completed.
-- 
https://code.launchpad.net/~stevenk/launchpad/check-job-status-on-accept/+merge/173396
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/check-job-status-on-accept into lp:launchpad.

=== modified file 'lib/lp/soyuz/model/queue.py'
--- lib/lp/soyuz/model/queue.py	2013-06-20 05:50:00 +0000
+++ lib/lp/soyuz/model/queue.py	2013-07-08 03:53:25 +0000
@@ -72,6 +72,7 @@
     ArrayContains,
     )
 from lp.services.features import getFeatureFlag
+from lp.services.job.interfaces.job import JobStatus
 from lp.services.librarian.browser import ProxiedLibraryFileAlias
 from lp.services.librarian.interfaces.client import DownloadFailed
 from lp.services.librarian.model import (
@@ -538,9 +539,13 @@
             raise QueueInconsistentStateError(
                 "Can't resurrect rejected syncs")
 
+        job = PlainPackageCopyJob.get(self.package_copy_job_id)
+        if job.status == JobStatus.COMPLETED:
+            raise QueueInconsistentStateError(
+                "Package sync job has already completed.")
+
         # Release the job hounds, Smithers.
         self.setAccepted()
-        job = PlainPackageCopyJob.get(self.package_copy_job_id)
         job.resume()
         job.celeryRunOnCommit()
         # The copy job will send emails as appropriate.  We don't

=== modified file 'lib/lp/soyuz/tests/test_packagecopyjob.py'
--- lib/lp/soyuz/tests/test_packagecopyjob.py	2013-06-20 05:50:00 +0000
+++ lib/lp/soyuz/tests/test_packagecopyjob.py	2013-07-08 03:53:25 +0000
@@ -50,7 +50,10 @@
     IPlainPackageCopyJobSource,
     )
 from lp.soyuz.interfaces.publishing import PackagePublishingStatus
-from lp.soyuz.interfaces.queue import IPackageUploadSet
+from lp.soyuz.interfaces.queue import (
+    IPackageUploadSet,
+    QueueInconsistentStateError,
+    )
 from lp.soyuz.interfaces.section import ISectionSet
 from lp.soyuz.interfaces.sourcepackageformat import (
     ISourcePackageFormatSelectionSet,
@@ -373,6 +376,16 @@
             name=spn).one()
         self.assertEqual(new_publication.packageupload, pu)
 
+    def test_copy_with_packageupload_and_completed_job(self):
+        spn = self.factory.getUniqueUnicode()
+        pcj = self.createCopyJob(spn, 'universe', 'web', '1.0-1', True)
+        pu = getUtility(IPackageUploadSet).getByPackageCopyJobIDs(
+            [pcj.id]).one()
+        removeSecurityProxy(pcj.job)._status = JobStatus.COMPLETED
+        self.assertRaisesWithContent(
+            QueueInconsistentStateError,
+            "Package sync job has already completed.", pu.acceptFromQueue)
+
     def test_target_ppa_non_release_pocket(self):
         # When copying to a PPA archive the target must be the release pocket.
         distroseries = self.factory.makeDistroSeries()