Code review comment for lp:~frankban/juju-gui/quickstart-bootstrap

*** Submitted:

Parse and validate the environment file/options.

Check if the given environment name exists,
ensure it does not use the local provider,
ensure it includes an admin-secret,
retrieve the admin-secret.

Tests: make check

R=gary.poster, matthew.scott
CC=
https://codereview.appspot.com/14669047

https://codereview.appspot.com/14669047/diff/1/Makefile
File Makefile (right):

https://codereview.appspot.com/14669047/diff/1/Makefile#newcode20
Makefile:20: JUJU_ENV ?= quickstart
On 2013/10/15 17:02:11, gary.poster wrote:
> So...this is used by make run to override the normal Python logic of
the
> command. I'm not quite sure of the value, but I'm fine with it.

You are right, since JUJU_ENV and the default env are already handled by
the application, this no longer makes sense. Fixed.

https://codereview.appspot.com/14669047/diff/1/quickstart/tests/helpers.py
File quickstart/tests/helpers.py (right):

https://codereview.appspot.com/14669047/diff/1/quickstart/tests/helpers.py#newcode17
quickstart/tests/helpers.py:17: """Tests helpers for the Juju Quickstart
plugin."""
On 2013/10/15 17:02:11, gary.poster wrote:
> Trivial: Test helpers

Done.

https://codereview.appspot.com/14669047/diff/1/quickstart/tests/test_manage.py
File quickstart/tests/test_manage.py (right):

https://codereview.appspot.com/14669047/diff/1/quickstart/tests/test_manage.py#newcode119
quickstart/tests/test_manage.py:119: and a it is also possible to
simulate an arbitrary environment name.
On 2013/10/15 17:02:11, gary.poster wrote:
> trivial: and it is

Done.

https://codereview.appspot.com/14669047/diff/1/quickstart/tests/test_utils.py
File quickstart/tests/test_utils.py (right):

https://codereview.appspot.com/14669047/diff/1/quickstart/tests/test_utils.py#newcode48
quickstart/tests/test_utils.py:48: self.assertEqual(2, retcode)
On 2013/10/15 17:02:11, gary.poster wrote:
> checking precise non-zero retcode seems a bit risky, even though it
probably is
> practically fine for something as ancient as ls.

Good suggestion, fixed.

https://codereview.appspot.com/14669047/diff/1/quickstart/tests/test_utils.py#newcode58
quickstart/tests/test_utils.py:58: self.assertEqual(127, retcode)
On 2013/10/15 17:02:11, gary.poster wrote:
> likewise

In this case this is safe because we return 127 ourselves.

https://codereview.appspot.com/14669047/diff/1/quickstart/utils.py
File quickstart/utils.py (right):

https://codereview.appspot.com/14669047/diff/1/quickstart/utils.py#newcode58
quickstart/utils.py:58: retcode, output, _ = call('juju', 'switch')
On 2013/10/15 17:02:11, gary.poster wrote:
> I wonder which is more fragile: relying on a regex of a stdout reply,
or looking
> at ~/.juju/current-environment and ~/.juju/environments.yaml[default]?
  This is
> certainly easier, so I suppose that's a compelling argument, given
that the
> answer to my question is not obvious to me.

As discussed, I will ask juju-core devs about
~/.juju/current-environment (is it reliable?, if not, is "juju switch
--format json" a good idea?).

https://codereview.appspot.com/14669047/