https://codereview.appspot.com/29980043/diff/40001/quickstart/app.py#newcode60
quickstart/app.py:60: cmds = [
ISTM that this is a good place to inform the user about what's
happening, e.g.: 'juju is not installed', 'sudo privileges required to
install juju'. It could be nice to give an idea about why we need sudo.
https://codereview.appspot.com/29980043/diff/40001/quickstart/app.py#newcode73
quickstart/app.py:73: raise ProgramExit('Juju is now installed; please
run `juju init` and '
I am not sure about exiting with an error here.
What if the user has a juju home properly set up but juju is not
installed? This is not uncommon, e.g. he removed the package, or he is
working in a lxc sharing his home directory. Moreover this code is never
reached if the envs.yaml is not properly set up, so we are just
preventing users with a configured juju home to proceed. I think we
should just continue with the normal execution here: in the future we
will guide the user setting up the environment.
Nice branch Matthew, thank you!
LGTM with changes (see below).
Will QA later.
https:/ /codereview. appspot. com/29980043/ diff/40001/ quickstart/ app.py
File quickstart/app.py (right):
https:/ /codereview. appspot. com/29980043/ diff/40001/ quickstart/ app.py# newcode60 app.py: 60: cmds = [
quickstart/
ISTM that this is a good place to inform the user about what's
happening, e.g.: 'juju is not installed', 'sudo privileges required to
install juju'. It could be nice to give an idea about why we need sudo.
https:/ /codereview. appspot. com/29980043/ diff/40001/ quickstart/ app.py# newcode66 app.py: 66: cmd.insert(0, 'sudo')
quickstart/
You can either add sudo in the cmds above (and make cmds a list of
tuples) or just "retcode, _, error = utils.call('sudo', *cmd)" below
(and make cmds a list of tuples :-). <shrug>
https:/ /codereview. appspot. com/29980043/ diff/40001/ quickstart/ app.py# newcode73 app.py: 73: raise ProgramExit('Juju is now installed; please
quickstart/
run `juju init` and '
I am not sure about exiting with an error here.
What if the user has a juju home properly set up but juju is not
installed? This is not uncommon, e.g. he removed the package, or he is
working in a lxc sharing his home directory. Moreover this code is never
reached if the envs.yaml is not properly set up, so we are just
preventing users with a configured juju home to proceed. I think we
should just continue with the normal execution here: in the future we
will guide the user setting up the environment.
https:/ /codereview. appspot. com/29980043/ diff/40001/ quickstart/ manage. py manage. py (right):
File quickstart/
https:/ /codereview. appspot. com/29980043/ diff/40001/ quickstart/ manage. py#newcode221 manage. py:221: print('ensuring juju and lxc are installed')
quickstart/
What do you think about making this a logging.debug call? I am not sure
about how much this is informative after the first run.
https:/ /codereview. appspot. com/29980043/