launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:process-job-source-cronspam into launchpad:master.

Commit message:
Avoid cronspam from process-job-source

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/388314

Log '%d job sources failed.' at INFO rather than ERROR, so that we don't get piles of cronspam from failures to acquire locks.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:process-job-source-cronspam into launchpad:master.

diff --git a/lib/lp/services/job/scripts/process_job_source.py b/lib/lp/services/job/scripts/process_job_source.py
index 4a81ecb..a8ff88b 100644
--- a/lib/lp/services/job/scripts/process_job_source.py
+++ b/lib/lp/services/job/scripts/process_job_source.py
@@ -169,5 +169,5 @@ class ProcessJobSource(LaunchpadScript):
                 if isinstance(handler, OopsHandler):
                     root_logger.removeHandler(handler)
         if failure_count:
-            self.logger.error('%d job sources failed.' % failure_count)
+            self.logger.info('%d job sources failed.' % failure_count)
             raise SilentLaunchpadScriptFailure(failure_count)
diff --git a/lib/lp/services/job/scripts/tests/test_process_job_source.py b/lib/lp/services/job/scripts/tests/test_process_job_source.py
index 52c7499..d70da7b 100644
--- a/lib/lp/services/job/scripts/tests/test_process_job_source.py
+++ b/lib/lp/services/job/scripts/tests/test_process_job_source.py
@@ -5,6 +5,10 @@
 
 __metaclass__ = type
 
+import os.path
+from textwrap import dedent
+
+from contrib.glock import GlobalLock
 import transaction
 from zope.component import getUtility
 
@@ -14,6 +18,7 @@ from lp.registry.interfaces.teammembership import (
     )
 from lp.services.config import config
 from lp.services.job.scripts import process_job_source
+from lp.services.scripts.base import LOCK_PATH
 from lp.services.scripts.tests import run_script
 from lp.testing import (
     login_person,
@@ -67,6 +72,28 @@ class ProcessJobSourceTest(TestCaseWithFactory):
             'source-IMembershipNotificationJobSource.lock.*')
         self.assertTextMatchesExpressionIgnoreWhitespace(expected, error)
 
+    def test_locked(self):
+        # If the script is already locked, running it logs the fact and exits
+        # non-zero, but doesn't log anything above INFO.
+        lock_file_path = os.path.join(
+            LOCK_PATH,
+            'launchpad-process-job-source-IMembershipNotificationJobSource'
+            '.lock')
+        lock = GlobalLock(lock_file_path)
+        lock.acquire()
+        try:
+            returncode, output, error = run_script(
+                self.script, ['IMembershipNotificationJobSource'],
+                expect_returncode=1)
+            expected = dedent('''\
+                INFO    Creating lockfile: {lock}
+                INFO    Lockfile {lock} in use
+                INFO    1 job sources failed.
+                ''').format(lock=lock_file_path)
+            self.assertTextMatchesExpressionIgnoreWhitespace(expected, error)
+        finally:
+            lock.release()
+
     def test_processed(self):
         # The script should output the number of jobs it processed.
         person = self.factory.makePerson(name='murdock')