launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/lazr.jobrunner/userErrorTypes into lp:lazr.jobrunner.

Commit message:
Support user_error_types even when security-proxied.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1070804 in lazr.jobrunner: "Error notification by mails doesn't describe the error"
  https://bugs.launchpad.net/lazr.jobrunner/+bug/1070804

For more details, see:
https://code.launchpad.net/~cjwatson/lazr.jobrunner/userErrorTypes/+merge/131248

JobRunner has an arrangement to cooperate with Launchpad to remove security proxies from job.retry_error_types, but it has no such arrangement for user_error_types.  As a result, user_error_types does not in fact work, as far as I can tell, leading to OOPSes that should be caught.

This change mirrors the code for retry_error_types.  It is a no-op on its own, and will require a subsequent change to Launchpad to take useful effect.
-- 
https://code.launchpad.net/~cjwatson/lazr.jobrunner/userErrorTypes/+merge/131248
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/lazr.jobrunner/userErrorTypes into lp:lazr.jobrunner.

=== modified file 'src/lazr/jobrunner/jobrunner.py'
--- src/lazr/jobrunner/jobrunner.py	2012-04-19 20:45:05 +0000
+++ src/lazr/jobrunner/jobrunner.py	2012-10-24 18:01:32 +0000
@@ -192,7 +192,7 @@
             try:
                 try:
                     self.runJob(job, fallback)
-                except job.user_error_types, e:
+                except self.userErrorTypes(job), e:
                     self.logger.info(
                         '%s failed with user error %r.'
                         % (self.job_str(job), e))
@@ -211,6 +211,9 @@
     def retryErrorTypes(self, job):
         return job.retry_error_types
 
+    def userErrorTypes(self, job):
+        return job.user_error_types
+
     @staticmethod
     @contextmanager
     def oopsMessage(message):

=== modified file 'src/lazr/jobrunner/tests/test_jobrunner.py'
--- src/lazr/jobrunner/tests/test_jobrunner.py	2012-04-19 19:58:33 +0000
+++ src/lazr/jobrunner/tests/test_jobrunner.py	2012-10-24 18:01:32 +0000
@@ -332,3 +332,25 @@
         runner.runJobHandleError(job)
         self.assertEqual(JobStatus.WAITING, job.status)
         self.assertEqual(1, job.attempt_count)
+
+    def test_runner_obeys_user_error_types_method(self):
+
+        class UserError(Exception):
+            pass
+
+        class UserErrorRunner(JobRunner):
+
+            def userErrorTypes(self, job):
+                return (UserError,)
+
+        runner = UserErrorRunner(
+            logger=self.logger, oops_config=self.oops_config)
+        job = FakeJob(1, UserError('no OOPS expected'))
+        self.assertFalse(job.notifyUserError_called)
+        runner.runJobHandleError(job)
+        self.assertEqual(0, self.oops_repository.oops_count)
+        self.assertEqual(
+            "<FakeJob> (ID 1) failed with user error "
+            "UserError('no OOPS expected',).",
+            self.log_handler.records[-1].msg)
+        self.assertTrue(job.notifyUserError_called)