Code review comment for lp:~adeuring/launchpad/lp-lazr.jobrunner

> Not landable yet because it assumes lazr.jobrunner is a development egg.
> "../lazr.jobrunner" should not be listed in buildout.cfg, and a version
> of lazr.jobrunner should be specified in versions.cfg.

Done.

>
> runJob may not need to be overridden in BaseJobRunner. If not, please
> remove it.

I think that we should keep this method: class runner.BaseRunnableJob
implements some required methods like notifyOops(), but it does not
implement start(), complete() etc. The latter methods are defined in
lp.services.job.model.job.Job -- and all of them are required by
lazr.jobrunner.runner.JobRunner. So, "IRunnableJob(job)" in

    def runJob(self, job):
        super(BaseJobRunner, self).runJob(IRunnableJob(job))

in BaseJobRunner ensures that the Zope machinery adapts the "incomplete"
job to IRunnableJob. OK, IRunnableJob(job) is also called in
JobRunner.runAll() and in TwistedJobRunner.runJobInSubprocess(), so it is
right now not strictly necessary in JobRunner.runJob() -- but keeping
JobRunner.runJob() makes it easier to remove runAll() later.

Alternatively, we could call IRunnableJob(job) in lazr.jobrunner, but I
am not sure if this makes sense.

> job_str may not need to be overridden in BaseJobRunner. If not, please
> remove it.

Right, this is no longer needed. Removed.