launchpad-reviewers team mailing list archive

["j.c.sackett" <jonathan.sackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/bug-expiration-doesnt-oops into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/bug-expiration-doesnt-oops/+merge/58412

Summary
=======
The bug expiration script wasn't working due to a permisions error, but because it wasn't logging the issue no Oops was raised.

The major parts of setting up the script and running the expiration job are now enclosed in try/excepts that log any errors to the Oops handler.

Preimplementation
=================
Spoke with Curtis Hovey

Implementation
==============
Added try/excepts around the initialization of the BugJanitor and the janitor running it's expiration job. The excepts are catchalls because we can't predict what might happen--anything expected should be handled within the routines called, this is a last-ditch "at least log it" measure.

Tests
=====
I cannot for the life of me figure out how to test a cronjob itself. The actual expiration code used by the cronjob is already tested, and the error actually occurs when the cronscript itself is run. If there is a utility to test cronscript/crontabs that I am unaware of, I am happy to implement tests for this.

QA
==
qa-untestable. This will only come into being if we encounter a new error. If necessary, we can alter security.cfg to reintroduce the error we originally discovered and see if it gets logged properly.

Lint
====
= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  cronscripts/expire-bugtasks.py

./cronscripts/expire-bugtasks.py
    16: '_pythonpath' imported but unused
    33: E222 multiple spaces after operator
-- 
https://code.launchpad.net/~jcsackett/launchpad/bug-expiration-doesnt-oops/+merge/58412
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/bug-expiration-doesnt-oops into lp:launchpad.

=== modified file 'cronscripts/expire-bugtasks.py'
--- cronscripts/expire-bugtasks.py	2010-10-21 20:05:52 +0000
+++ cronscripts/expire-bugtasks.py	2011-04-20 00:01:15 +0000
@@ -47,13 +47,23 @@
             # Avoid circular import.
             from lp.registry.interfaces.distribution import IDistributionSet
             target = getUtility(IDistributionSet).getByName('ubuntu')
-        janitor = BugJanitor(
-            log=self.logger, target=target, limit=self.options.limit)
-        janitor.expireBugTasks(self.txn)
+        try:
+            janitor = BugJanitor(
+                log=self.logger, target=target, limit=self.options.limit)
+        except Exception as error:
+            # We use a catchall here (and in the next block) because we don't
+            # know (and don't care) about the particular error--we'll just
+            # log it to as an Oops.
+            self.logger.error(
+                'An error occured getting the janitor: %s' % error)
+        try:
+            janitor.expireBugTasks(self.txn)
+        except Exception as error:
+            self.logger.error(
+                'An error occured trying to expire bugtasks: %s' % error)
 
 
 if __name__ == '__main__':
     script = ExpireBugTasks(
         'expire-bugtasks', dbuser=config.malone.expiration_dbuser)
     script.lock_and_run()
-