Code review comment for lp:~frankban/juju-quickstart/uncommitted-bundles

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

Please take a look.

https://codereview.appspot.com/247800043/diff/1/quickstart/app.py
File quickstart/app.py (right):

https://codereview.appspot.com/247800043/diff/1/quickstart/app.py#newcode659
quickstart/app.py:659: 'deployed Juju GUI charm ({}): requested bundle
deployment '
On 2015/06/05 12:28:41, rharding wrote:
> should we hint at a juju-gui upgrade here?

Good idea, done.

https://codereview.appspot.com/247800043/diff/1/quickstart/app.py#newcode663
quickstart/app.py:663: print('requesting uncommitted deployment of {}
with the following '
On 2015/06/05 12:28:41, rharding wrote:
> "uncommitted deployment" to "uncomitted loading"? deployment just
strikes me as
> very...deployed.

Done.

https://codereview.appspot.com/247800043/diff/1/quickstart/app.py#newcode679
quickstart/app.py:679: return response['Token']
On 2015/06/05 12:28:41, rharding wrote:
> does this open the gui url as it does when you just run
juju-quickstart without
> the deployment?

This token, if returned, is used to build the GUI URL.
Opening the GUI on the browser is done by manage.run.

https://codereview.appspot.com/247800043/diff/1/quickstart/jujutools.py
File quickstart/jujutools.py (right):

https://codereview.appspot.com/247800043/diff/1/quickstart/jujutools.py#newcode60
quickstart/jujutools.py:60: if not jujugui.is_promulgated(charm_ref):
On 2015/06/05 12:28:42, rharding wrote:
> oic, so we assume. I still wonder about using version.js vs
is_promulgated.

Well, some of the features we check (e.g. bundle deployment, uncommitted
bundles or new API endpoints) must be implemented principally on the
server (because qucikstart interacts mostly with the GUI server via its
API), and that's why in the cases we know the promulgated revision we
can safely check for their availability.
Given this is quickstart, I'd expect that we are dealing with
promulgated GUI charms 99% of the times, as you already mentioned. For
the remaining cases we trust the user and we expect her to be smart
enough to handle possible (and graceful) quickstart errors, which is the
only bad thing that can happen if the current customized charm does not
support a requested feature.

https://codereview.appspot.com/247800043/

« Back to merge proposal