Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(193)

Issue 247800043: Add support for uncommitted bundles.

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 8 months ago by frankban
Modified:
10 years, 8 months ago
Reviewers:
rharding, mp+261200
Visibility:
Public.

Description

Add support for uncommitted bundles. Introduce the -u/--uncommitted flag, which enables uncommitted bundle support. Improve output messages and tokens handling. Also update jujubundlelib dep to latest version. TESTS: `make fcheck` and wait a while for the functional tests to complete. QA: - deploy bundles as usual: `devenv/bin/juju-quickstart mediawiki-single`; - deploy uncommitted bundles: `devenv/bin/juju-quickstart -u u/openstack-charmers/openstack`; https://code.launchpad.net/~frankban/juju-quickstart/uncommitted-bundles/+merge/261200 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 9

Patch Set 2 : Add support for uncommitted bundles. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+334 lines, -57 lines) Patch
A [revision details] View 1 chunk +2 lines, -0 lines 0 comments Download
M quickstart/__init__.py View 1 chunk +1 line, -1 line 0 comments Download
M quickstart/app.py View 1 chunk +62 lines, -6 lines 0 comments Download
M quickstart/juju.py View 1 chunk +9 lines, -0 lines 0 comments Download
M quickstart/jujugui.py View 2 chunks +28 lines, -0 lines 0 comments Download
M quickstart/jujutools.py View 2 chunks +2 lines, -5 lines 0 comments Download
M quickstart/manage.py View 4 chunks +31 lines, -21 lines 0 comments Download
M quickstart/settings.py View 2 chunks +7 lines, -2 lines 0 comments Download
M quickstart/tests/functional/test_functional.py View 1 chunk +8 lines, -0 lines 0 comments Download
M quickstart/tests/test_app.py View 1 chunk +99 lines, -16 lines 0 comments Download
M quickstart/tests/test_juju.py View 1 chunk +12 lines, -0 lines 0 comments Download
M quickstart/tests/test_jujugui.py View 2 chunks +46 lines, -0 lines 0 comments Download
M quickstart/tests/test_manage.py View 4 chunks +23 lines, -2 lines 0 comments Download
M tox.ini View 4 chunks +4 lines, -4 lines 0 comments Download

Messages

Total messages: 3
frankban
Please take a look.
10 years, 8 months ago (2015-06-05 10:13:20 UTC) #1
rharding
LGTM, I think most of my questions are answered in some way or another. I ...
10 years, 8 months ago (2015-06-05 12:28:42 UTC) #2
frankban
10 years, 8 months ago (2015-06-05 13:15:32 UTC) #3
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#newco...
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.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b