I ran into some problems merging in db-devel due to some changes in LaunchpadCronScript. I switched to using properties to make it easy for subclasses to override the values without interfering with __init__() logic. Instead of an incremental diff, here is a full diff of the affected files. === added file 'cronscripts/process-job-source.py' --- cronscripts/process-job-source.py 1970-01-01 00:00:00 +0000 +++ cronscripts/process-job-source.py 2010-10-04 04:01:36 +0000 @@ -0,0 +1,69 @@ +#!/usr/bin/python -S +# +# Copyright 2009, 2010 Canonical Ltd. This software is licensed under the +# GNU Affero General Public License version 3 (see the file LICENSE). + +"""Handle jobs for a specified job source class.""" + +__metaclass__ = type + +import sys + +import _pythonpath +from twisted.python import log + +from canonical.config import config +from lp.services.job import runner +from lp.services.job.runner import JobCronScript + + +class ProcessJobSource(JobCronScript): + """Run jobs for a specified job source class.""" + usage = ( + "Usage: %prog [options] JOB_SOURCE\n\n" + "For more help, run:\n" + " cronscripts/process-job-source-groups.py --help") + + def __init__(self): + super(ProcessJobSource, self).__init__() + # The fromlist argument is necessary so that __import__() + # returns the bottom submodule instead of the top one. + module = __import__(self.config_section.module, + fromlist=[self.job_source_name]) + self.source_interface = getattr(module, self.job_source_name) + + @property + def config_name(self): + return self.job_source_name + + @property + def name(self): + return 'process-job-source-%s' % self.job_source_name + + @property + def dbuser(self): + return self.config_section.dbuser + + @property + def runner_class(self): + runner_class_name = getattr( + self.config_section, 'runner_class', 'JobRunner') + # Override attributes that are normally set in __init__(). + return getattr(runner, runner_class_name) + + def handle_options(self): + if len(self.args) != 1: + self.parser.print_help() + sys.exit(1) + self.job_source_name = self.args[0] + super(ProcessJobSource, self).handle_options() + + def main(self): + if self.options.verbose: + log.startLogging(sys.stdout) + super(ProcessJobSource, self).main() + + +if __name__ == '__main__': + script = ProcessJobSource() + script.lock_and_run() === modified file 'lib/lp/services/job/runner.py' --- lib/lp/services/job/runner.py 2010-09-20 09:48:58 +0000 +++ lib/lp/services/job/runner.py 2010-10-04 04:01:36 +0000 @@ -77,7 +77,6 @@ # We redefine __eq__ and __ne__ here to prevent the security proxy # from mucking up our comparisons in tests and elsewhere. - def __eq__(self, job): return ( self.__class__ is removeSecurityProxy(job.__class__) @@ -337,13 +336,12 @@ """Run Jobs via twisted.""" def __init__(self, job_source, dbuser, logger=None, error_utility=None): - env = {'PYTHONPATH': os.environ['PYTHONPATH'], - 'PATH': os.environ['PATH']} - lp_config = os.environ.get('LPCONFIG') - if lp_config is not None: - env['LPCONFIG'] = lp_config + env = {'PATH': os.environ['PATH']} + for name in ('PYTHONPATH', 'LPCONFIG'): + if name in os.environ: + env[name] = os.environ[name] starter = main.ProcessStarter( - packages=('twisted', 'ampoule'), env=env) + packages=('_pythonpath', 'twisted', 'ampoule'), env=env) super(TwistedJobRunner, self).__init__(logger, error_utility) self.job_source = job_source import_name = '%s.%s' % ( @@ -367,7 +365,8 @@ transaction.commit() job_id = job.id deadline = timegm(job.lease_expires.timetuple()) - self.logger.debug('Running %r, lease expires %s', job, job.lease_expires) + self.logger.debug( + 'Running %r, lease expires %s', job, job.lease_expires) deferred = self.pool.doWork( RunJobCommand, job_id = job_id, _deadline=deadline) def update(response): @@ -442,14 +441,17 @@ class JobCronScript(LaunchpadCronScript): """Base class for scripts that run jobs.""" - def __init__(self, runner_class=JobRunner, test_args=None, - script_name=None): - self.dbuser = getattr(config, self.config_name).dbuser - if script_name is None: - script_name = self.config_name + config_name = None + + def __init__(self, runner_class=JobRunner, test_args=None, name=None): super(JobCronScript, self).__init__( - script_name, self.dbuser, test_args) - self.runner_class = runner_class + name=name, dbuser=None, test_args=test_args) + self._runner_class = runner_class + + @property + def runner_class(self): + """Enable subclasses to override with command-line arguments.""" + return self._runner_class def job_counts(self, jobs): """Return a list of tuples containing the job name and counts.""" @@ -458,8 +460,18 @@ counts[job.__class__.__name__] += 1 return sorted(counts.items()) + @property + def config_section(self): + return getattr(config, self.config_name) + def main(self): - errorlog.globalErrorUtility.configure(self.config_name) + section = self.config_section + if (getattr(section, 'error_dir', None) is not None + and getattr(section, 'oops_prefix', None) is not None + and getattr(section, 'copy_to_zlog', None) is not None): + # If the three variables are not set, we will let the error + # utility default to using the [error_reports] config. + errorlog.globalErrorUtility.configure(self.config_name) job_source = getUtility(self.source_interface) runner = self.runner_class.runFromSource( job_source, self.dbuser, self.logger) === modified file 'lib/lp/services/scripts/base.py' --- lib/lp/services/scripts/base.py 2010-09-26 13:18:24 +0000 +++ lib/lp/services/scripts/base.py 2010-10-04 04:01:36 +0000 @@ -89,6 +89,7 @@ self._log_unhandled_exceptions_level += 1 return func(self, *args, **kw) except Exception: + raise if self._log_unhandled_exceptions_level == 1: # self.logger is setup in LaunchpadScript.__init__() so # we can use it. @@ -152,22 +153,21 @@ useful in test scripts. """ if name is None: - self.name = self.__class__.__name__.lower() - else: - self.name = name - - self.dbuser = dbuser - - if self.description is None: - description = self.__doc__ - else: - description = self.description + self._name = self.__class__.__name__.lower() + else: + self._name = name + + self._dbuser = dbuser # The construction of the option parser is a bit roundabout, but # at least it's isolated here. First we build the parser, then # we add options that our logger object uses, then call our # option-parsing hook, and finally pull out and store the # supplied options and args. + if self.description is None: + description = self.__doc__ + else: + description = self.description self.parser = OptionParser(usage=self.usage, description=description) scripts.logger_options(self.parser, default=self.loglevel) @@ -177,9 +177,23 @@ "profiling stats in FILE.")) self.add_my_options() self.options, self.args = self.parser.parse_args(args=test_args) - self.logger = scripts.logger(self.options, name) - - self.lockfilepath = os.path.join(LOCK_PATH, self.lockfilename) + + # Enable subclasses to easily override these __init__() + # arguments using command-line arguments. + self.handle_options() + + def handle_options(self): + self.logger = scripts.logger(self.options, self.name) + + @property + def name(self): + """Enable subclasses to override with command-line arguments.""" + return self._name + + @property + def dbuser(self): + """Enable subclasses to override with command-line arguments.""" + return self._dbuser # # Hooks that we expect users to redefine. @@ -226,6 +240,10 @@ """ return "launchpad-%s.lock" % self.name + @property + def lockfilepath(self): + return os.path.join(LOCK_PATH, self.lockfilename) + def setup_lock(self): """Create lockfile. @@ -275,8 +293,10 @@ @log_unhandled_exception_and_exit def run(self, use_web_security=False, implicit_begin=True, - isolation=ISOLATION_LEVEL_DEFAULT): + isolation=None): """Actually run the script, executing zcml and initZopeless.""" + if isolation is None: + isolation = ISOLATION_LEVEL_DEFAULT self._init_zca(use_web_security=use_web_security) self._init_db(implicit_begin=implicit_begin, isolation=isolation) @@ -339,18 +359,8 @@ """Logs successful script runs in the database.""" def __init__(self, name=None, dbuser=None, test_args=None): - """Initialize, and sys.exit() if the cronscript is disabled. - - Rather than hand editing crontab files, cronscripts can be - enabled and disabled using a config file. - - The control file location is specified by - config.canonical.cron_control_url. - """ super(LaunchpadCronScript, self).__init__(name, dbuser, test_args) - name = self.name - # Configure the IErrorReportingUtility we use with defaults. # Scripts can override this if they want. globalErrorUtility.configure() @@ -361,10 +371,22 @@ # WARN and higher log messages should generate OOPS reports. logging.getLogger().addHandler(OopsHandler(name)) + def run(self, use_web_security=False, implicit_begin=True, + isolation=None): + """Run if the cronscript is not disabled by the control file. + + Rather than hand editing crontab files, cronscripts can be + enabled and disabled using a config file. + + The control file location is specified by + config.canonical.cron_control_url. + """ enabled = cronscript_enabled( - config.canonical.cron_control_url, name, self.logger) + config.canonical.cron_control_url, self.name, self.logger) if not enabled: sys.exit(0) + super(LaunchpadCronScript, self).run( + use_web_security, implicit_begin, isolation) @log_unhandled_exception_and_exit def record_activity(self, date_started, date_completed):