Code review comment for lp:~mbp/launchpad/612171-diff-generation-spam

Revision history for this message
Martin Pool (mbp) wrote :

How about something like this? (The tests don't pass yet.)

=== modified file 'lib/lp/code/model/branchmergeproposaljob.py'
--- lib/lp/code/model/branchmergeproposaljob.py 2011-10-13 09:56:53 +0000
+++ lib/lp/code/model/branchmergeproposaljob.py 2011-10-18 07:58:02 +0000
@@ -332,6 +332,15 @@
     """Raised if the the preview diff is not ready to run."""

+class BranchHasPendingWrites(Exception):
+ """Raised if the branch can't be processed because a write is pending.
+
+ In this case the operation can usually be retried in a while.
+
+ See bug 612171.
+ """
+
+
 class UpdatePreviewDiffJob(BranchMergeProposalJobDerived):
     """A job to update the preview diff for a branch merge proposal.

@@ -346,6 +355,16 @@

     user_error_types = (UpdatePreviewDiffNotReady, )

+ retry_error_types = [BranchHasPendingWrites]
+
+ @classmethod
+ def create(cls, bmp):
+ """See `IMergeProposalCreationJob`."""
+ job = BranchMergeProposalJob(
+ bmp, cls.class_job_type, {})
+ job.max_retries = 20
+ return cls(job)
+
     def checkReady(self):
         """Is this job ready to run?"""
         bmp = self.branch_merge_proposal
@@ -356,8 +375,8 @@
             raise UpdatePreviewDiffNotReady(
                 'The target branch has no revisions.')
         if bmp.source_branch.pending_writes:
- raise UpdatePreviewDiffNotReady(
- 'The source branch has pending writes.')
+ raise BranchHasPendingWrites(
+ 'The source branch has pending writes')

     @staticmethod
     @contextlib.contextmanager
@@ -386,11 +405,8 @@

     def getErrorRecipients(self):
         """Return a list of email-ids to notify about user errors."""
- # All these failures are either obvious user errors in the web ui (eg
- # nothing in the branch), or Launchpad internal errors (eg the branch
- # is being written to). See bug 612171. Either way, the user can't
- # be anything but annoyed by mail about it. -- mbp 2011-10-13
- return []
+ registrant = self.branch_merge_proposal.registrant
+ return format_address_for_person(registrant)

 class CreateMergeProposalJob(BaseRunnableJob):

« Back to merge proposal