Code review comment for lp:~bcsaller/pyjuju/lxc-lib

Revision history for this message
Kapil Thangavelu (hazmat) wrote :

Gustavo has done a great review on details, some additional high level things...

[0]

While building on top of lxc-lib, i ended up needing two additional functionalities, listing all containers with a given prefix, and finding out if a given container is running. The current usage of the running attribute is broken for usages where the container was not started by the same instance of an lxccontainer class.

i used this implementation to satisfy both
http://bazaar.launchpad.net/~hazmat/ensemble/local-provider/view/head:/ensemble/providers/lxc/container.py

I'm fine with this branch being merged without, i can incorporate this fix directly into lib/lxc this in a subsequent local-dev branch.

[1] Pep8 nitpicks

http://paste.ubuntu.com/682624/

[2] Re gustavo's _cmd comments, it would be nice if this was made public or pushed into a separate process module inside of ensemble/lib.. I'm currently using subprocess.check_output/subprocess.check_call very regularly, which are bit suboptimal in light of gustavo's comments.

cheers,

Kapil

review: Approve

« Back to merge proposal