launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/launchpad/pcj-more-often-bug-770721-pre into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #770721 in Launchpad itself: "Schedule PackageCopyJob to run more frequently."
  https://bugs.launchpad.net/launchpad/+bug/770721

For more details, see:
https://code.launchpad.net/~allenap/launchpad/pcj-more-often-bug-770721-pre/+merge/74293

Make --exclude work with process-job-source-groups.py. This is needed
by a succession of future branches:

1. Change IPlainPackageCopyJobSource to run on ackee and stop running
   it on loganberry. This will be done with --exclude.

2. Change IPlainPackageCopyJobSource to be in the FREQUENT job source
   group.

3. Remove the --exclude from the crontab on loganberry.

This should ensure that the job continues to run without interruption,
and without any time-dependent changes needing to be made, like
changing a crontab at the same time that a Launchpad change is
deployed.

-- 
https://code.launchpad.net/~allenap/launchpad/pcj-more-often-bug-770721-pre/+merge/74293
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/pcj-more-often-bug-770721-pre into lp:launchpad.

=== modified file 'cronscripts/process-job-source-groups.py'
--- cronscripts/process-job-source-groups.py	2010-09-30 20:39:02 +0000
+++ cronscripts/process-job-source-groups.py	2011-09-06 19:49:24 +0000
@@ -16,6 +16,7 @@
 import _pythonpath
 
 from canonical.config import config
+from canonical.launchpad.helpers import english_list
 from lp.services.propertycache import cachedproperty
 from lp.services.scripts.base import LaunchpadCronScript
 
@@ -68,8 +69,9 @@
         # Then, exclude job sources.
         for source in self.options.excluded_job_sources:
             if source not in selected_job_sources:
-                self.logger.info('%r is not in job source groups %s'
-                                  % (source, self.options.groups))
+                self.logger.info(
+                    '%r is not in %s' % (
+                        source, english_list(selected_groups, "or")))
             else:
                 selected_job_sources.remove(source)
         # Process job sources.

=== modified file 'lib/lp/registry/tests/test_process_job_sources_cronjob.py'
--- lib/lp/registry/tests/test_process_job_sources_cronjob.py	2011-02-25 11:53:04 +0000
+++ lib/lp/registry/tests/test_process_job_sources_cronjob.py	2011-09-06 19:49:24 +0000
@@ -8,6 +8,7 @@
 import transaction
 from zope.component import getUtility
 
+from canonical.config import config
 from canonical.launchpad.scripts.tests import run_script
 from canonical.testing import LaunchpadScriptLayer
 from lp.registry.interfaces.teammembership import (
@@ -18,6 +19,7 @@
     login_person,
     TestCaseWithFactory,
     )
+from lp.testing.matchers import DocTestMatches
 
 
 class ProcessJobSourceTest(TestCaseWithFactory):
@@ -72,6 +74,16 @@
     layer = LaunchpadScriptLayer
     script = 'cronscripts/process-job-source-groups.py'
 
+    def getJobSources(self, *groups):
+        sources = config['process-job-source-groups'].job_sources
+        sources = (source.strip() for source in sources.split(','))
+        sources = (source for source in sources if source in config)
+        if len(groups) != 0:
+            sources = (
+                source for source in sources
+                if config[source].crontab_group in groups)
+        return sorted(set(sources))
+
     def tearDown(self):
         super(ProcessJobSourceGroupsTest, self).tearDown()
         self.layer.force_dirty_database()
@@ -119,3 +131,25 @@
             error)
         self.assertIn('DEBUG   MembershipNotificationJob sent email', error)
         self.assertIn('Ran 1 MembershipNotificationJob jobs.', error)
+
+    def test_exclude(self):
+        # Job sources can be excluded with a --exclude switch.
+        args = ["MAIN"]
+        for source in self.getJobSources("MAIN"):
+            args.extend(("--exclude", source))
+        returncode, output, error = run_script(self.script, args)
+        expected = "INFO    Creating lockfile: ...\n"
+        self.assertThat(error, DocTestMatches(expected))
+
+    def test_exclude_non_existing_group(self):
+        # If a job source specified by --exclude does not exist the script
+        # continues, logging a short info message about it.
+        args = ["MAIN"]
+        for source in self.getJobSources("MAIN"):
+            args.extend(("--exclude", source))
+        args.extend(("--exclude", "BobbyDazzler"))
+        returncode, output, error = run_script(self.script, args)
+        expected = (
+            "INFO    Creating lockfile: ...\n"
+            "INFO    'BobbyDazzler' is not in MAIN\n")
+        self.assertThat(error, DocTestMatches(expected))