Code review comment for lp:~hazmat/pyjuju/local-origin-passthrough

Revision history for this message
Benjamin Saller (bcsaller) wrote :

[3]

132 - _customize_container(self.customize_script, self.rootfs)
133 + _, output = _customize_container(self.customize_script, self.rootfs)
134 +
135 + if self.customize_log:
136 + with open(self.customize_log, "w") as fh:
137 + fh.write(output)

Its very possible to write a customize_log location which is in the container but not writeable by the caller. In cases like this opening a temp file and using [sudo mv tmp customize_log] might make sense. Then again I suppose there is a single supported use of this and its already written so I don't feel strongly.

[4]

+ apt-get install --force-yes -y bzr tmux sudo python-software-properties python-yaml

I am reminded reminded that we have a bug around hook execution needing the DEBIAN_FRONTEND=noninteractive. I think we should do this in this file as well

DEBIAN_FRONTEND=noninteractive
APT_OPTIONS="-o Dpkg::Options::="--force-confnew --force-yes -fuy"

apt-get install $APT_OPTIONS ...

might make sense in this file as well.

[5]
175 + # light weight checkout is significantly faster, no history though
176 + bzr co --lightweight $JUJU_ORIGIN /usr/lib/juju/juju

This strikes me as very much the correct thing to so here.

[6] The private address relation stuff seems like a good idea, but I'll be more pleased when the unit-get stuff you did lands. This branch still can't run the examples as they stand now.

review: Approve

« Back to merge proposal