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

Issue 6977043: Updated the charm to serve Juju GUI releases.

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 3 months ago by frankban
Modified:
11 years, 3 months ago
Reviewers:
mp+140745
Visibility:
Public.

Description

Updated the charm to serve Juju GUI releases. I had a pre-implementation call with Gary and some really useful help from Brad. Details: Replaced the juju-gui-branch config option with a new juju-gui-source one: here you can specify where to deploy the GUI from (stable, trunk, a branch, a specific version of stable/trunk). Added a bunch of functions in utils.py: they help parsing the juju-gui-source option, retrieving the URL of a release, etc. Added tests for the utility functions above. Changed the fetch/build function in utils.py, so that now a release tarball is always used to install the GUI. It can be downloaded from Launchpad or created using "make distfile" from a juju-gui checkout. Also reorganized the fetch/build code: separated each of the fetch/build functions into two different functions (one for the GUI, one for the API). Updated the install and the config-changed hooks to conform to the new fetch/setup functions (mentioned above). Updated the config-changed hook to reflect changes to the options, install hook and utils.py. Added one functional test specific to deploying from a branch. The other tests now default to the latest stable release. Created and uploaded Juju GUI trunk/stable releases. Updated the stop hook: now the function in utils.py is called with the argument it expects. Overall code lint and clean up. All those changes result in a big diff (~820 lines), sorry about that. https://code.launchpad.net/~frankban/charms/precise/juju-gui/bug-1088618-serve-releases/+merge/140745 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 2

Patch Set 2 : Updated the charm to serve Juju GUI releases. #

Total comments: 10

Patch Set 3 : Updated the charm to serve Juju GUI releases. #

Patch Set 4 : Updated the charm to serve Juju GUI releases. #

Patch Set 5 : Updated the charm to serve Juju GUI releases. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+470 lines, -106 lines) Patch
M README.md View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
A [revision details] View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
M config.yaml View 1 2 3 1 chunk +11 lines, -3 lines 0 comments Download
M config/nginx.conf.template View 1 2 3 1 chunk +12 lines, -8 lines 0 comments Download
M hooks/config-changed View 1 2 3 5 chunks +28 lines, -29 lines 0 comments Download
M hooks/install View 1 2 3 3 chunks +13 lines, -7 lines 0 comments Download
M hooks/start View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
M hooks/stop View 2 chunks +7 lines, -1 line 0 comments Download
M hooks/utils.py View 1 2 3 9 chunks +140 lines, -43 lines 0 comments Download
M tests/deploy.test View 1 2 3 4 chunks +13 lines, -3 lines 0 comments Download
M tests/test_utils.py View 1 2 4 chunks +240 lines, -11 lines 0 comments Download

Messages

Total messages: 14
frankban
Please take a look.
11 years, 3 months ago (2012-12-19 18:05:10 UTC) #1
gary.poster
This looks great Francesco. :-) I suggest that you ignore my bzr: idea (see below). ...
11 years, 3 months ago (2012-12-19 19:30:37 UTC) #2
gary.poster
Good news and bad news. The good news is that the deployment took three minutes ...
11 years, 3 months ago (2012-12-20 01:02:21 UTC) #3
gary.poster
On 2012/12/20 01:02:21, gary.poster wrote: > Good news and bad news. The good news is ...
11 years, 3 months ago (2012-12-20 03:32:06 UTC) #4
frankban
On 2012/12/20 03:32:06, gary.poster wrote: > The unit tests all passed just fine, for what ...
11 years, 3 months ago (2012-12-20 10:08:50 UTC) #5
frankban
Please take a look.
11 years, 3 months ago (2012-12-20 10:15:52 UTC) #6
teknico
Land with changes. Very nice refactoring job and new code, thanks. A few small changes ...
11 years, 3 months ago (2012-12-20 13:09:13 UTC) #7
gary.poster
I was just reviewing where we were and had two comments. No change to my ...
11 years, 3 months ago (2012-12-20 13:25:22 UTC) #8
frankban
Hi Nicola, thanks for the review. https://codereview.appspot.com/6977043/diff/8001/hooks/config-changed File hooks/config-changed (right): https://codereview.appspot.com/6977043/diff/8001/hooks/config-changed#newcode50 hooks/config-changed:50: # Restarting of ...
11 years, 3 months ago (2012-12-20 13:28:53 UTC) #9
frankban
Please take a look.
11 years, 3 months ago (2012-12-20 13:29:24 UTC) #10
teknico
https://codereview.appspot.com/6977043/diff/8001/hooks/config-changed File hooks/config-changed (right): https://codereview.appspot.com/6977043/diff/8001/hooks/config-changed#newcode50 hooks/config-changed:50: # Restarting of the gui and api services is ...
11 years, 3 months ago (2012-12-20 13:38:35 UTC) #11
frankban
https://codereview.appspot.com/6977043/diff/8001/hooks/config-changed File hooks/config-changed (right): https://codereview.appspot.com/6977043/diff/8001/hooks/config-changed#newcode50 hooks/config-changed:50: # Restarting of the gui and api services is ...
11 years, 3 months ago (2012-12-20 13:44:51 UTC) #12
frankban
Please take a look.
11 years, 3 months ago (2012-12-20 18:03:47 UTC) #13
frankban
11 years, 3 months ago (2012-12-20 18:12:37 UTC) #14
*** Submitted:

Updated the charm to serve Juju GUI releases.

I had a pre-implementation call with Gary and
some really useful help from Brad.

Details:

Replaced the juju-gui-branch config option with a new juju-gui-source
one: here you can specify where to deploy the GUI from (stable, trunk, a
branch, a specific version of stable/trunk).

Added a bunch of functions in utils.py: they help parsing the
juju-gui-source option, retrieving the URL of a release, etc.

Added tests for the utility functions above.

Changed the fetch/build function in utils.py, so that now a release
tarball is always used to install the GUI. It can be downloaded from
Launchpad or created using "make distfile" from a juju-gui checkout.
Also reorganized the fetch/build code: separated each of the fetch/build
functions into two different functions (one for the GUI, one for the API).

Updated the install and the config-changed hooks to conform to the
new fetch/setup functions (mentioned above).

Updated the config-changed hook to reflect changes to the options,
install hook and utils.py.

Added one functional test specific to deploying from a branch.
The other tests now default to the latest stable release.

Created and uploaded Juju GUI trunk/stable releases.

Updated the stop hook: now the function in utils.py is called with the
argument it expects.

Overall code lint and clean up.

All those changes result in a big diff (~820 lines), sorry about that.

R=gary.poster, teknico
CC=
https://codereview.appspot.com/6977043
Sign in to reply to this message.

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