Code review comment for lp:~abentley/launchpad/ampoulejob

Revision history for this message
Aaron Bentley (abentley) wrote :

= Summary =
Modify the twisted job runner to use a subprocess.

== Proposed fix ==
Use the Ampoule process pool. Note that this implementation does not provide
parallelism, because the ParallelLimitedTaskConsumer is paramaterized to run
1 parallel process.

== Pre-implementation notes ==
Preimplementation discussion with thumper, mwhudson.

== Implementation details ==
Make ampoule a dependency.

Because process set-up should happen in a subprocess, with Ampoule, remove the
setUp concept from JobCronScript, and instead provide a context manager.
Configure the globalErrorUtility there. Use the context manager in JobRunner.

Add an Amp command called RunJobCommand. Add a JobRunnerProcess class that can
provide RunJobCommand. Have it enter the context when a connection is made,
and exit it when the connection is lost. (These should happen only once in the
JobRunnerProcess's lifecycle.)

Subclass RunJobCommand as UpdatePreviewDiffProcess. Provide
UpdatePreviewDiffJob.amp so that TwistedJobRunner.runFromSource can find the
right process class to use.

Update runAll to start the process pool, using the specified job_amp kind.
Provide a customized BOOTSTRAP to invoke scripts.execute_zcml_for_scripts.

Change getTaskSource to return runJobInSubprocess, which uses the process pool
to run the job.

Update TwistedJobRunner.runFromSource to clean up SIGCHLD signal handlers after
reactor.run.

== Tests ==
bin/test test_update_preview_diffs

== Demo and Q/A ==
Have a LOSA update the staging config to use --twisted, and ensure the log
says it ran under Twisted.

« Back to merge proposal