Code review comment for lp://staging/~fwereade/pyjuju/upstartify-services

[0]

machine/unit -> _get_container no longer uses zookeeper_hosts

[1]

machine/unit -> AgentProcessProtocol has been obsoleted.

it would be good to have the upstart config capture stderr/stdout to a file location, for debugging output, which the agentprocessprotocol would do as a python exception.

[2]

for local dev the user should also be set, ideally to that of the user for things that don't need root (ie. storage server, zk server).

[3]
+ # Reuse the lxc flag for tests needing sudo
+ test_start_stop.skip = test_namespacing.skip = run_lxc_tests()

These flags should be set physically next to the test the value is set on.

[5]

i'm a little concerned by the branch in that we're no longer running processes to a large extent, instead substituting mocking, so effectively decreasing the unit test real world coverage.

specifically most of the service unit tests that would actually launch a real process, have been given over for mocks. those tests where valuable in discovering problems with pythonpath or parent directories in the past.

i'd recommend additional skip enabled tests for the machine unit deployment.

Its seems like several unit deployment tests where removed that still need testing, specifically around container directory creation/destruction.