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

Issue 191940043: Support deploying bundles from custom users.

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 4 months ago by frankban
Modified:
9 years, 4 months ago
Reviewers:
bac, mp+245011, jeff.pihach
Visibility:
Public.

Description

Support deploying bundles from custom users. Also add the charmstore-url charm option so that the new GUI can properly connect to the charm store. Update the deployer version (we use a fork for now) and the GUI server so that both the user and the password are provided when calling the deployer blocking functions. If you are curious, the deployer merge proposal is here: https://code.launchpad.net/~frankban/juju-deployer/guiserver-auth/+merge/244984 Tests: `make unittest`. QA: - bootstrap an environment with juju 1.21; - deploy this charm by using `make deploy`; - wait for the charm to be ready; - switch to the most recent GUI: `juju set juju-gui juju-gui-source=develop`; - while waiting, create a new juju user: `juju user add new-user --generate`; - copy to clipboard the password included in the resulting new-user.jenv file; - wait for the trunk release to be ready; - visit the GUI and log in using "new-user" and the copied password; - ensure the charm browser is correctly displayed, meaning the charm properly connects to the charm store; - search for a bundle in the browser and deploy it: you should get this error: "An error occurred while deploying the bundle: Invalid relation in config, service 1 not found, rel 1 <-> 0 Invalid relation in config, service 0 not found, rel 1 <-> 0" or similar... This is another issue. Dragging and dropping an old bundle from your desktop should instead work; - destroy the environment. Done, thank you! https://code.launchpad.net/~frankban/charms/trusty/juju-gui/new-deployer-auth/+merge/245011 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 4

Patch Set 2 : Support deploying bundles from custom users. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+59 lines, -33 lines) Patch
M Makefile View 1 chunk +1 line, -1 line 0 comments Download
A [revision details] View 1 chunk +2 lines, -0 lines 0 comments Download
M config.yaml View 1 chunk +6 lines, -0 lines 0 comments Download
M config/config.js.template View 1 chunk +3 lines, -1 line 0 comments Download
D deps/juju-deployer-0.4.1.tar.gz View 0 chunks +-1 lines, --1 lines 0 comments Download
A deps/juju-deployer-0.4.3.tar.gz View 0 chunks +-1 lines, --1 lines 0 comments Download
M hooks/backend.py View 1 chunk +3 lines, -2 lines 0 comments Download
M hooks/utils.py View 2 chunks +2 lines, -1 line 0 comments Download
M server-requirements.pip View 1 chunk +5 lines, -1 line 0 comments Download
M server/guiserver/bundles/base.py View 2 chunks +4 lines, -2 lines 0 comments Download
M server/guiserver/tests/bundles/test_base.py View 3 chunks +4 lines, -4 lines 0 comments Download
M tests/test_backends.py View 2 chunks +4 lines, -3 lines 0 comments Download
M tests/test_utils.py View 12 chunks +27 lines, -20 lines 0 comments Download

Messages

Total messages: 5
frankban
Please take a look.
9 years, 4 months ago (2014-12-17 16:31:46 UTC) #1
jeff.pihach
LGTM with no QA https://codereview.appspot.com/191940043/diff/1/Makefile File Makefile (right): https://codereview.appspot.com/191940043/diff/1/Makefile#newcode17 Makefile:17: JUJUTEST = yes | juju-test ...
9 years, 4 months ago (2014-12-17 16:59:53 UTC) #2
frankban
Please take a look. https://codereview.appspot.com/191940043/diff/1/Makefile File Makefile (right): https://codereview.appspot.com/191940043/diff/1/Makefile#newcode17 Makefile:17: JUJUTEST = yes | juju-test ...
9 years, 4 months ago (2014-12-17 17:14:47 UTC) #3
bac
Code LGTM. Wow, I like the parallelized QA instructions. Nice touch. On to QA now.
9 years, 4 months ago (2014-12-17 19:36:22 UTC) #4
bac
9 years, 4 months ago (2014-12-17 20:06:31 UTC) #5
QA-OK

Thanks!
Sign in to reply to this message.

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