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

Excerpts from Benjamin Saller's message of Tue Oct 04 20:26:25 UTC 2011:
> Review: Approve
>
> [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.

Customize is only invoked once for master template container, not per container
in practice since its not used for clones. To make it easy to discover and find
this file we end up putting it into data-dir/units/master-customize.log. Putting
it in the container would detract from easy discovery. In practice most of the
issues with setting up the local provider where localized to the output from
this script, and keeping it outside the container helps ensure it survives
a destroy-environment if its needed for additional debugging. iotw, i think its
in a good spot for now.

>
> [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.
>

Sounds good, thanks.

>
>
> [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.
>

Indeed, if needed they can be easily bound on a per container basis. I'm a
little concerned that this will end up stale because customize is only invoked
once, but i don't see any other easy options here, and it has a nice side
benefit ensuring consistency across a deploy.

>
> [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.

Yeah.. but they where already broken for local deploy before the branch.. they
unit-get cli branch should be merged shortly.

thanks for the review.
cheers,
Kapil