Code review comment for lp:~frankban/juju-quickstart/charm-url-warning

Revision history for this message
Francesco Banconi (frankban) wrote :

Reviewers: mp+196244_code.launchpad.net,

Message:
Please take a look.

Description:
Improve charm URL handling.

Validate the user provided GUI charm URL.
Also add a missing test for complete coverage
(obsessive-compulsive mode on).

Tests: `make check`.

QA:

1)
Provide invalid charm URLs, the program
should immediately stop the execution
and exit with an error, e.g.:

.venv/bin/python juju-quickstart --gui-charm-url invalid

.venv/bin/python juju-quickstart --gui-charm-url
local:precise/juju-gui-80

.venv/bin/python juju-quickstart --gui-charm-url
http:~juju-gui/precise/juju-gui-80

.venv/bin/python juju-quickstart --gui-charm-url cs:precise/juju-gui-1
bundle:~jorge/mediawiki-simple/4/mediawiki-simple

.venv/bin/python juju-quickstart --gui-charm-url cs:saucy/juju-gui-80

2)
Run the program passing a customized charm URL, e.g.:

.venv/bin/python juju-quickstart --gui-charm-url
cs:~juju-gui/precise/juju-gui-128

You should see the "using a customized juju-gui charm" warning
printed during the service deployment step.

Re-execute the command above: quickstart should reuse the
service in the environment and the warning is printed again.

Destroy the environment.

3)
Now manually deploy an outdated version of the GUI charm:

(sudo) juju bootstrap
juju deploy cs:precise/juju-gui-79

Run quickstart:
.venv/bin/python juju-quickstart

You should see the "charm is outdated" warning, then quickstart
waits for the outdated GUI to be deployed and ready.

Destroy the environment.

4)
Run quickstart normally:
.venv/bin/python juju-quickstart
The last official GUI charm (cs:precise/juju-gui-80)
is installed and no warnings are logged.

Destroy the environment.
Done, thank you!

https://code.launchpad.net/~frankban/juju-quickstart/charm-url-warning/+merge/196244

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/30760043/

Affected files (+637, -39 lines):
   A [revision details]
   M quickstart/__init__.py
   M quickstart/app.py
   A quickstart/charms.py
   M quickstart/manage.py
   M quickstart/settings.py
   M quickstart/tests/test_app.py
   A quickstart/tests/test_charms.py
   M quickstart/tests/test_manage.py
   M quickstart/tests/test_utils.py
   M quickstart/utils.py

« Back to merge proposal