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

Revision history for this message
Gary Poster (gary) wrote :

LGTM with an extremely small suggestion that you are welcome to ignore.
It is interesting and convenient that so many of the commands have
(seemingly) reliably parsable output. The branch has nice and
interesting tests. Thank you!

https://codereview.appspot.com/14441074/diff/5001/quickstart/app.py
File quickstart/app.py (right):

https://codereview.appspot.com/14441074/diff/5001/quickstart/app.py#newcode49
quickstart/app.py:49: retcode, output, error = utils.call('juju',
'status', '-e', env_name)
I'm OK with this for expediency, but wouldn't the API (Kapil's library)
be better in a revision?

[later] Oh, it's YAML, yeah? I guess that's OK. I think it would be
nice to add a comment to that effect, but others might disagree. Just a
suggestion.

https://codereview.appspot.com/14441074/diff/5001/quickstart/app.py#newcode70
quickstart/app.py:70: 'juju', 'api-endpoints', '-e', env_name,
'--format', 'json')
yay, --format json

https://codereview.appspot.com/14441074/diff/5001/quickstart/utils.py
File quickstart/utils.py (right):

https://codereview.appspot.com/14441074/diff/5001/quickstart/utils.py#newcode69
quickstart/utils.py:69: # Switch to using "juju switch --raw" in order
to avoid the fragility
oh, cool.

https://codereview.appspot.com/14441074/

« Back to merge proposal