Code review comment for lp:~abentley/launchpad/ampoule-0.1.1

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

= Summary =
Update to Ampoule 0.2.0.

== Proposed fix ==
This branch updates ampoule and also adds a version for pyOpenSSL, which is a
new Ampoule dependency. This new version of ampoule has many changes that I
suggested, to push as much code upstream as I thought was architecturally
sound.

== Pre-implementation notes ==
None

== Implementation details ==
Instead of having a separate AMPChild subclass for every JobSource, a new
ampoule feature allows us to parameterize the JobRunnerProcess. We supply a
full path to the JobSource. In JobRunnerProcess, we import this and use it. I
used 'exec' for this, because the import module looked crazy complicated.

Instead of supplying our own BOOTSTRAP code in order to provide a bit more
startup code, a new ampoule feature lets us simply implement the context
manager protocol in JobRunnerProcess.

Instead of subclassing pool.ProcessPool to use SIGHUP, ampoule now lets us
parameterize the timout_signal.

Instead of converting our leases, which are datetimes, into timeout durations,
a new ampoule feature lets us supply them as deadlines.

job_class was renamed to job_source where this was a better description of its
usage.

== Tests ==
bin/test -vt test_runner -t update_preview_diffs

== Demo and Q/A ==
No visible change.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/services/job/tests/test_runner.py
  lib/lp/services/job/runner.py
  versions.cfg
  lib/lp/code/model/branchmergeproposaljob.py

== Pyflakes notices ==

lib/lp/services/job/runner.py
    240: undefined name 'job_source'

^^^ This is because of the exec.

== Pylint notices ==

lib/lp/services/job/runner.py
    35: [F0401] Unable to import 'lazr.delegates' (No module named delegates)
    239: [W0122, JobRunnerProcess.__init__] Use of the exec statement
    240: [E0602, JobRunnerProcess.__init__] Undefined variable 'job_source'

^^^ This is because of the exec.

lib/lp/code/model/branchmergeproposaljob.py
    22: [F0401] Unable to import 'lazr.delegates' (No module named delegates)
    23: [F0401] Unable to import 'lazr.enum' (No module named enum)

« Back to merge proposal