Merge lp:~wgrant/launchpad/reject-not-accepted into lp:launchpad

[William Grant]

Fix NascentUpload.do_reject to not send an erroneous Accepted email.

Fix NascentUpload.do_reject to not send an erroneous Accepted email.

It would previously do so if the upload made it to Done before an exception occurred (eg. because build creation failed).

[Colin Watson (cjwatson)]

This seems to change do_reject to only send the mail, and not actually change the status (PU.notify doesn't do that). That seems odd. Can you explain what's going on there?

review: Needs Information

[William Grant (wgrant)]

On 05/01/16 01:57, Colin Watson wrote:
> Review: Needs Information
>
> This seems to change do_reject to only send the mail, and not
> actually change the status (PU.notify doesn't do that). That seems
> odd. Can you explain what's going on there?

The transaction is about to be aborted, so the only side-effect that can
depend on the status is the email. Since we can't reliably set the
status, I opted to be consistent and rely solely on the override.

[Colin Watson (cjwatson)]

OK, I hadn't traced things through to confirm that do_reject is always followed by an abort (and was put off by the original nascentupload.txt test that you removed; I now realise that it goes through a path that doesn't involve the abort). Now that I have, I agree with you.

review: Approve

=== modified file 'lib/lp/archiveuploader/nascentupload.py'
--- lib/lp/archiveuploader/nascentupload.py	2014-08-13 07:47:43 +0000
+++ lib/lp/archiveuploader/nascentupload.py	2015-12-30 23:59:31 +0000
@@ -50,6 +50,7 @@
     FromExistingOverridePolicy,
     SourceOverride,
     )
+from lp.soyuz.enums import PackageUploadStatus
 from lp.soyuz.interfaces.binarypackagename import IBinaryPackageNameSet
 from lp.soyuz.interfaces.component import IComponentSet
 from lp.soyuz.interfaces.queue import QueueInconsistentStateError
@@ -755,17 +756,9 @@
         if not self.queue_root:
             self.queue_root = self._createQueueEntry()
 
-        # Avoid cyclic imports.
-        from lp.soyuz.interfaces.queue import QueueInconsistentStateError
-        try:
-            self.queue_root.setRejected()
-        except QueueInconsistentStateError:
-            # These exceptions are ignored, we want to force the rejected
-            # state.
-            pass
-
         with open(self.changes.filepath, "r") as changes_file_object:
             self.queue_root.notify(
+                status=PackageUploadStatus.REJECTED,
                 summary_text=self.rejection_message,
                 changes_file_object=changes_file_object, logger=self.logger)
 
 
=== modified file 'lib/lp/archiveuploader/tests/nascentupload.txt'
--- lib/lp/archiveuploader/tests/nascentupload.txt	2015-04-21 10:03:15 +0000
+++ lib/lp/archiveuploader/tests/nascentupload.txt	2015-12-30 23:59:31 +0000
@@ -489,17 +489,6 @@
     cannot upload the same version within the same distribution. You
     have to modify the source version and re-upload.
 
-We rely on process-upload transaction rollback to not store bogus
-queue entry in the database.
-
-    >>> print ed_src_dup.queue_root.status.name
-    REJECTED
-
-    >>> from lp.soyuz.enums import PackageUploadStatus
-    >>> hoary.getPackageUploads(
-    ...     status=PackageUploadStatus.REJECTED, name=u"ed").count()
-    1
-
 
 Staged Source and Binary upload with multiple binaries
 ......................................................
@@ -717,6 +706,7 @@
 content, it should have all the required fields except the
 'dsc_standards_version':
 
+    >>> from lp.soyuz.enums import PackageUploadStatus
     >>> inst_queue = hoary.getPackageUploads(
     ...     PackageUploadStatus.NEW, name=u'test75874', exact_match=True)[0]
     >>> inst_spr = inst_queue.sources[0].sourcepackagerelease