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

Issue 8727047: Refactor Charm for Sandbox support

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 11 months ago by bcsaller
Modified:
10 years, 11 months ago
Reviewers:
mp+159265, matthew.scott, gary.poster
Visibility:
Public.

Description

Refactor Charm for Sandbox support This adds a composition system for creating backend object. Backend implement start(), stop() and install() methods. A backend is composed of many mixins and each mixin will implement any/of of those methods and all will be called. Backends additionally provide for collecting property values from each mixin into a single final property on the backend. There is also a feature for determining if configuration values have changed between old and new configurations so we can selectively take action. Using these features we add support for Sandbox'd deployments, limiting support for adding apt repositories and the ability to fetch a juju-gui release from a URL specified in a configuration property. https://code.launchpad.net/~bcsaller/charms/precise/juju-gui/sandbox-charm/+merge/159265 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 27

Patch Set 2 : Refactor Charm for Sandbox support #

Total comments: 2

Patch Set 3 : Refactor Charm for Sandbox support #

Unified diffs Side-by-side diffs Delta from patch set Stats (+479 lines, -252 lines) Patch
M .bzrignore View 1 chunk +1 line, -0 lines 0 comments Download
A [revision details] View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M config.yaml View 1 2 chunks +20 lines, -1 line 0 comments Download
M config/config.js.template View 1 chunk +1 line, -0 lines 0 comments Download
A hooks/backend.py View 1 2 1 chunk +257 lines, -0 lines 0 comments Download
M hooks/bootstrap_utils.py View 1 2 chunks +4 lines, -0 lines 0 comments Download
M hooks/config-changed View 1 2 chunks +13 lines, -109 lines 0 comments Download
M hooks/install View 1 2 chunks +15 lines, -61 lines 0 comments Download
M hooks/start View 1 chunk +3 lines, -27 lines 0 comments Download
M hooks/stop View 1 chunk +4 lines, -2 lines 0 comments Download
M hooks/utils.py View 11 chunks +118 lines, -35 lines 0 comments Download
M revision View 1 1 chunk +1 line, -1 line 0 comments Download
A tests/test_backends.py View 1 chunk +39 lines, -0 lines 0 comments Download
M tests/test_utils.py View 3 chunks +1 line, -16 lines 0 comments Download

Messages

Total messages: 7
bcsaller
Please take a look.
10 years, 11 months ago (2013-04-17 00:52:45 UTC) #1
gary.poster
LGTM with changes as discussed below. This is a big improvement in readability for our ...
10 years, 11 months ago (2013-04-17 18:41:52 UTC) #2
bcsaller
Thanks for the review, good notes. I'm currently doing a test run post change and ...
10 years, 11 months ago (2013-04-17 19:34:41 UTC) #3
bcsaller
Please take a look.
10 years, 11 months ago (2013-04-17 20:16:28 UTC) #4
matthew.scott
LGTM - thanks for the branch! https://codereview.appspot.com/8727047/diff/3003/hooks/backend.py File hooks/backend.py (right): https://codereview.appspot.com/8727047/diff/3003/hooks/backend.py#newcode162 hooks/backend.py:162: api = legacy_juju() ...
10 years, 11 months ago (2013-04-17 20:48:48 UTC) #5
bcsaller
Thanks for the reviews, submitting. https://codereview.appspot.com/8727047/diff/3003/hooks/backend.py File hooks/backend.py (right): https://codereview.appspot.com/8727047/diff/3003/hooks/backend.py#newcode162 hooks/backend.py:162: api = legacy_juju() and ...
10 years, 11 months ago (2013-04-17 20:50:46 UTC) #6
bcsaller
10 years, 11 months ago (2013-04-17 20:52:07 UTC) #7
*** Submitted:

Refactor Charm for Sandbox support

This adds a composition system for creating backend object.
Backend implement start(), stop() and install() methods. A backend
is composed of many mixins and each mixin will implement any/of
of those methods and all will be called. Backends additionally 
provide for collecting property values from each mixin into a single 
final property on the backend. There is also a feature for determining 
if configuration values have changed between old and new configurations
so we can selectively take action.

Using these features we add support for Sandbox'd deployments, 
limiting support for adding apt repositories and the ability 
to fetch a juju-gui release from a URL specified in a configuration 
property.

R=gary.poster, matthew.scott
CC=
https://codereview.appspot.com/8727047
Sign in to reply to this message.

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