No longer add the juju-gui PPA by default:
the external repository is added only if required,
i.e. if the legacy server is used or if a branch
is passed to juju-gui-source.
The only missing bit to make the charm work well
from behind a firewall AFAICT is avoiding the release
to be downloaded from Launchpad.
Also included the shelltoolbox file in the charm:
unfortunately the python-shelltoolbox package is
not available on precise. On the other hand, this
allows for getting rid of the bootstrap_utils.py
file, and the install hook now feels cleaner.
Refactoring + some magic removal on the backend
framework. Now it should be less surprising, and
also allows for more customizations, e.g. what
I did in the install method.
Also added missing tests for the backend framework:
those were required in order to increase our control
over what's really happening in the backend "hooks".
Switched to the builtin Tornado server by default.
This diff is very big, I am sorry, but:
- you can ignore the bootstrap_utils removal;
- you can ignore the shelltoolbox.py file: it is
just a copy of the one present in the raring
python-shelltoolbox package;
- a lot of code is tests, the rest of the code
should be quite easy to follow.
QA:
`make deploy` and watch the logs:
- no PPA should be installed by default;
- the deployment succeeds and the GUI works well;
switch to builtin-server=false and watch the logs:
- the PPA is installed (and then haproxy, apache...);
- the config-change hook exits without errors and
the GUI works well.
Tests:
`make unittest`
(I ran the functional tests myself).
Yeah, I was curious about that dependency too. I found that curl is used
by fetch_gui_release to download a release tarball from Launchpad. So,
formally it belongs here: added a comment describing what curl is used
for.
Done thank you, and yes, my first implementation included that kwarg,
but then I realized it was unnecessary.
https://codereview.appspot.com/14433049/diff/1/hooks/backend.py#newcode310
hooks/backend.py:310: """Execute the charm starting steps."""
On 2013/10/07 17:01:08, gary.poster wrote:
> A subtle thing, but any one of these would read a bit better to me, as
ranked
> from my personal highest to lowest preference.
> Execute the charm's "start" steps.
> Execute the charm's starting steps.
> Execute the charm-starting steps.
https://codereview.appspot.com/14433049/diff/1/hooks/shelltoolbox.py#newcode3
hooks/shelltoolbox.py:3: # This file is part of python-shell-toolbox.
On 2013/10/07 17:01:08, gary.poster wrote:
> I included comments about the source in hooks/charmhelpers.py. Check
what I did
> there. If you like it, include it here; otherwise, feel free to
ignore it.
*** Submitted:
Avoid installing from PPA if not required.
No longer add the juju-gui PPA by default:
the external repository is added only if required,
i.e. if the legacy server is used or if a branch
is passed to juju-gui-source.
The only missing bit to make the charm work well
from behind a firewall AFAICT is avoiding the release
to be downloaded from Launchpad.
Also included the shelltoolbox file in the charm:
unfortunately the python-shelltoolbox package is
not available on precise. On the other hand, this
allows for getting rid of the bootstrap_utils.py
file, and the install hook now feels cleaner.
Refactoring + some magic removal on the backend
framework. Now it should be less surprising, and
also allows for more customizations, e.g. what
I did in the install method.
Also added missing tests for the backend framework:
those were required in order to increase our control
over what's really happening in the backend "hooks".
Switched to the builtin Tornado server by default.
This diff is very big, I am sorry, but: shelltoolbox package;
- you can ignore the bootstrap_utils removal;
- you can ignore the shelltoolbox.py file: it is
just a copy of the one present in the raring
python-
- a lot of code is tests, the rest of the code
should be quite easy to follow.
QA: server= false and watch the logs:
`make deploy` and watch the logs:
- no PPA should be installed by default;
- the deployment succeeds and the GUI works well;
switch to builtin-
- the PPA is installed (and then haproxy, apache...);
- the config-change hook exits without errors and
the GUI works well.
Tests:
`make unittest`
(I ran the functional tests myself).
Thank you!
R=gary.poster, rharding /codereview. appspot. com/14433049
CC=
https:/
https:/ /codereview. appspot. com/14433049/ diff/1/ hooks/backend. py
File hooks/backend.py (right):
https:/ /codereview. appspot. com/14433049/ diff/1/ hooks/backend. py#newcode116 py:116: debs = ('curl',)
hooks/backend.
On 2013/10/07 17:01:08, gary.poster wrote:
> Does this belong somewhere else now? Or can it be deleted?
Yeah, I was curious about that dependency too. I found that curl is used
by fetch_gui_release to download a release tarball from Launchpad. So,
formally it belongs here: added a comment describing what curl is used
for.
https:/ /codereview. appspot. com/14433049/ diff/1/ hooks/backend. py#newcode229 py:229: If reverse is True, the method is called on the
hooks/backend.
reversed list of objects.
On 2013/10/07 17:01:08, gary.poster wrote:
> Delete this line? (functionality is gone)
Done thank you, and yes, my first implementation included that kwarg,
but then I realized it was unnecessary.
https:/ /codereview. appspot. com/14433049/ diff/1/ hooks/backend. py#newcode310 py:310: """Execute the charm starting steps."""
hooks/backend.
On 2013/10/07 17:01:08, gary.poster wrote:
> A subtle thing, but any one of these would read a bit better to me, as
ranked
> from my personal highest to lowest preference.
> Execute the charm's "start" steps.
> Execute the charm's starting steps.
> Execute the charm-starting steps.
Done.
https:/ /codereview. appspot. com/14433049/ diff/1/ hooks/backend. py#newcode314 py:314: """Execute the charm stopping steps."""
hooks/backend.
On 2013/10/07 17:01:08, gary.poster wrote:
> Perhaps the following?
> ----
> Execute the charm's "stop" steps.
> Iterates through the mixins in reverse order.
Done.
https:/ /codereview. appspot. com/14433049/ diff/1/ hooks/backend. py#newcode318 py:318: """Execute the charm removal steps."""
hooks/backend.
On 2013/10/07 17:01:08, gary.poster wrote:
> WDYT?
> ---
> Execute the charm removal steps.
> Iterates through the mixins in reverse order.
Done.
https:/ /codereview. appspot. com/14433049/ diff/1/ hooks/shelltool box.py box.py (right):
File hooks/shelltool
https:/ /codereview. appspot. com/14433049/ diff/1/ hooks/shelltool box.py# newcode3 box.py: 3: # This file is part of python- shell-toolbox. ers.py. Check
hooks/shelltool
On 2013/10/07 17:01:08, gary.poster wrote:
> I included comments about the source in hooks/charmhelp
what I did
> there. If you like it, include it here; otherwise, feel free to
ignore it.
Good idea, done.
https:/ /codereview. appspot. com/14433049/