launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/queue-no-dry-run into lp:launchpad.

Commit message:
Remove unused arguments from PackageUpload.{acceptFromQueue,rejectFromQueue}.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/queue-no-dry-run/+merge/269907

Remove unused arguments from PackageUpload.{acceptFromQueue,rejectFromQueue}.  These were only used by the obsolete command-line queue tool and by a couple of tests that imitated it.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/queue-no-dry-run into lp:launchpad.

=== modified file 'lib/lp/archiveuploader/tests/nascentupload-announcements.txt'
--- lib/lp/archiveuploader/tests/nascentupload-announcements.txt	2015-08-25 14:05:24 +0000
+++ lib/lp/archiveuploader/tests/nascentupload-announcements.txt	2015-09-02 12:57:49 +0000
@@ -644,42 +644,6 @@
     >>> len(msgs)
     0
 
-The PackageUpload record created by NascentUpload will be in the NEW
-state if the package was not previously uploaded.  When accepted by an
-archive admin using the queue tool, the queue tool will call
-PackageUpload.notify() directly.  One of the arguments it can specify is
-dry_run, which when True will not send any emails.  It will also log at
-the INFO level what it /would/ have sent.
-
-    >>> random_package_upload = hoary.getPackageUploads()[0]
-    >>> random_package_upload.notify(dry_run=True, logger=FakeLogger())
-    DEBUG Building recipients list.
-    ...
-    DEBUG Would have sent a mail:
-    DEBUG   Subject: [ubuntu/hoary] bar 1.0-6 (Accepted)
-    DEBUG   Sender: Root <root@localhost>
-    DEBUG   Recipients: Celso Providelo <celso.providelo@xxxxxxxxxxxxx>
-    DEBUG   Bcc: Root <root@localhost>
-    DEBUG   Body:
-    DEBUG bar (1.0-6) breezy; urgency=low
-    ...
-    DEBUG Announcing to hoary-announce@xxxxxxxxxxxxxxxx
-    ...
-    DEBUG You are receiving this email because you are the most recent person
-    DEBUG listed in this package's changelog.
-    DEBUG Would have sent a mail:
-    DEBUG   Subject: [ubuntu/hoary] bar 1.0-6 (Accepted)
-    DEBUG   Sender: Celso Providelo <cprov@xxxxxxxxxx>
-    DEBUG   Recipients: hoary-announce@xxxxxxxxxxxxxxxx
-    DEBUG   Bcc: Root <root@localhost>, bar_derivatives@xxxxxxxxxxxxxxxxxxxxxx
-    ...
-
-No emails generated:
-
-    >>> msgs = pop_notifications()
-    >>> len(msgs)
-    0
-
 Uploads with UTF-8 characters in email addresses in the changes file are
 permitted, but RFC2047-encoded.  UTF-8 in the mail content is preserved.
 

=== modified file 'lib/lp/soyuz/interfaces/queue.py'
--- lib/lp/soyuz/interfaces/queue.py	2013-07-25 11:58:55 +0000
+++ lib/lp/soyuz/interfaces/queue.py	2015-09-02 12:57:49 +0000
@@ -355,7 +355,7 @@
     @export_write_operation()
     @call_with(user=REQUEST_USER)
     @operation_for_version("devel")
-    def acceptFromQueue(logger=None, dry_run=False, user=None):
+    def acceptFromQueue(user=None):
         """Call setAccepted, do a syncUpdate, and send notification email.
 
          * Grant karma to people involved with the upload.
@@ -366,7 +366,7 @@
         comment=TextLine(title=_("Rejection comment"), required=False))
     @call_with(user=REQUEST_USER)
     @operation_for_version("devel")
-    def rejectFromQueue(user, logger=None, dry_run=False, comment=None):
+    def rejectFromQueue(user, comment=None):
         """Call setRejected, do a syncUpdate, and send notification email."""
 
     def realiseUpload(logger=None):

=== modified file 'lib/lp/soyuz/mail/packageupload.py'
--- lib/lp/soyuz/mail/packageupload.py	2015-08-26 13:38:05 +0000
+++ lib/lp/soyuz/mail/packageupload.py	2015-09-02 12:57:49 +0000
@@ -426,9 +426,8 @@
     def __init__(self, subject, template_name, recipients, from_address,
                  action, info, blamee, spr, bprs, customfiles, archive,
                  distroseries, pocket, summary_text=None, changes=None,
-                 changesfile_content=None, dry_run=False,
-                 announce_from_address=None, previous_version=None,
-                 logger=None):
+                 changesfile_content=None, announce_from_address=None,
+                 previous_version=None, logger=None):
         super(PackageUploadMailer, self).__init__(
             subject, template_name, recipients, from_address,
             notification_type="package-upload")
@@ -443,7 +442,6 @@
         self.pocket = pocket
         self.changes = changes
         self.changesfile_content = changesfile_content
-        self.dry_run = dry_run
         self.logger = logger
         self.announce_from_address = announce_from_address
         self.previous_version = previous_version
@@ -597,10 +595,7 @@
         """See `BaseMailer`."""
         ctrl = super(PackageUploadMailer, self).generateEmail(
             email, recipient, force_no_attachments=force_no_attachments)
-        if self.dry_run:
-            debug(self.logger, "Would have sent a mail:")
-        else:
-            debug(self.logger, "Sent a mail:")
+        debug(self.logger, "Sent a mail:")
         debug(self.logger, "  Subject: %s" % ctrl.subject)
         debug(self.logger, "  Sender: %s" % ctrl.from_addr)
         debug(self.logger, "  Recipients: %s" % ", ".join(ctrl.to_addrs))
@@ -612,11 +607,3 @@
                 line = line.decode('utf-8', 'replace')
             debug(self.logger, line)
         return ctrl
-
-    def sendOne(self, email, recipient):
-        """See `BaseMailer`."""
-        if self.dry_run:
-            # Just generate the email for the sake of debugging output.
-            self.generateEmail(email, recipient)
-        else:
-            super(PackageUploadMailer, self).sendOne(email, recipient)

=== modified file 'lib/lp/soyuz/model/queue.py'
--- lib/lp/soyuz/model/queue.py	2015-08-25 14:05:24 +0000
+++ lib/lp/soyuz/model/queue.py	2015-09-02 12:57:49 +0000
@@ -529,7 +529,7 @@
         # should probably give karma but that needs more work to
         # fix here.
 
-    def _acceptNonSyncFromQueue(self, logger=None, dry_run=False):
+    def _acceptNonSyncFromQueue(self):
         """Accept a "regular" upload from the queue.
 
         This is the normal case, for uploads that are not delayed and are not
@@ -550,9 +550,7 @@
         # We explicitly allow unsigned uploads here since the .changes file
         # is pulled from the librarian which are stripped of their
         # signature just before being stored.
-        self.notify(
-            logger=logger, dry_run=dry_run,
-            changes_file_object=changes_file_object)
+        self.notify(changes_file_object=changes_file_object)
         self.syncUpdate()
 
         # If this is a single source upload we can create the
@@ -572,17 +570,17 @@
         # Give some karma!
         self._giveKarma()
 
-    def acceptFromQueue(self, logger=None, dry_run=False, user=None):
+    def acceptFromQueue(self, user=None):
         """See `IPackageUpload`."""
         if self.package_copy_job is None:
-            self._acceptNonSyncFromQueue(logger, dry_run)
+            self._acceptNonSyncFromQueue()
         else:
             self._acceptSyncFromQueue()
         if bool(getFeatureFlag('auditor.enabled')):
             client = AuditorClient()
             client.send(self, 'packageupload-accepted', user)
 
-    def rejectFromQueue(self, user, logger=None, dry_run=False, comment=None):
+    def rejectFromQueue(self, user, comment=None):
         """See `IPackageUpload`."""
         self.setRejected()
         if self.package_copy_job is not None:
@@ -608,7 +606,6 @@
         # We allow unsigned uploads since they come from the librarian,
         # which are now stored unsigned.
         self.notify(
-            logger=logger, dry_run=dry_run,
             changes_file_object=changes_file_object, summary_text=summary_text)
         self.syncUpdate()
         if bool(getFeatureFlag('auditor.enabled')):
@@ -899,8 +896,7 @@
         else:
             return None
 
-    def notify(self, summary_text=None, changes_file_object=None,
-               logger=None, dry_run=False):
+    def notify(self, summary_text=None, changes_file_object=None, logger=None):
         """See `IPackageUpload`."""
         status_action = {
             PackageUploadStatus.NEW: 'new',
@@ -920,8 +916,7 @@
             self.builds, self.customfiles, self.archive, self.distroseries,
             self.pocket, summary_text=summary_text, changes=changes,
             changesfile_content=changesfile_content,
-            changesfile_object=changes_file_object, dry_run=dry_run,
-            logger=logger)
+            changesfile_object=changes_file_object, logger=logger)
         mailer.sendAll()
 
     @property

=== modified file 'lib/lp/soyuz/tests/test_distroseriesqueue_dist_upgrader.py'
--- lib/lp/soyuz/tests/test_distroseriesqueue_dist_upgrader.py	2013-05-28 01:24:33 +0000
+++ lib/lp/soyuz/tests/test_distroseriesqueue_dist_upgrader.py	2015-09-02 12:57:49 +0000
@@ -72,11 +72,10 @@
         transaction.commit()
         # Reject from accepted queue (unlikely, would normally be from
         # unapproved or new).
-        upload.queue_root.rejectFromQueue(
-            self.factory.makePerson(), logger=self.logger)
+        upload.queue_root.rejectFromQueue(self.factory.makePerson())
         self.assertEqual("REJECTED", upload.queue_root.status.name)
         # Accept from rejected queue (also unlikely, but only for testing).
-        upload.queue_root.acceptFromQueue(logger=self.logger)
+        upload.queue_root.acceptFromQueue()
         self.assertEqual("ACCEPTED", upload.queue_root.status.name)
 
     def test_bad_upload_remains_in_accepted(self):