Avoid installing from PPA if not required.

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:
- 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).


Thank you!

https://code.launchpad.net/~frankban/charms/precise/juju-gui/support-firewall/+merge/189645

(do not edit description out of merge proposal)

[frankban, 2013-10-07 15:50:28]

Please take a look.

[gary.poster, 2013-10-07 17:01:08]

Wow!  Great tests, and very nice improvement.  Code LGTM.  I will check with
Rick as to which of us will QA.

Thank you,

Gary

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
hooks/backend.py:116: debs = ('curl',)
Does this belong somewhere else now? Or can it be deleted?

https://codereview.appspot.com/14433049/diff/1/hooks/backend.py#newcode229
hooks/backend.py:229: If reverse is True, the method is called on the reversed
list of objects.
Delete this line? (functionality is gone)

https://codereview.appspot.com/14433049/diff/1/hooks/backend.py#newcode310
hooks/backend.py:310: """Execute the charm starting steps."""
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/backend.py#newcode314
hooks/backend.py:314: """Execute the charm stopping steps."""
Perhaps the following?
----
Execute the charm's "stop" steps.

Iterates through the mixins in reverse order.

https://codereview.appspot.com/14433049/diff/1/hooks/backend.py#newcode318
hooks/backend.py:318: """Execute the charm removal steps."""
WDYT?
---
Execute the charm removal steps.

Iterates through the mixins in reverse order.

https://codereview.appspot.com/14433049/diff/1/hooks/shelltoolbox.py
File hooks/shelltoolbox.py (right):

https://codereview.appspot.com/14433049/diff/1/hooks/shelltoolbox.py#newcode3
hooks/shelltoolbox.py:3: # This file is part of python-shell-toolbox.
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.

[rharding, 2013-10-07 17:15:20]

LGTM QA ok

For QA I did a deploy, ssh'd into the box to verify that the ppa, apache/haproxy
packages were not installed.

Checked the gui worked fine.

Then changed the config for the server. SSH'd back into the box to verify that
packages were then installed. Gui still worked fine.

[frankban, 2013-10-08 07:49:37]

*** 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:
- 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).


Thank you!

R=gary.poster, rharding
CC=
https://codereview.appspot.com/14433049

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
hooks/backend.py:116: debs = ('curl',)
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
hooks/backend.py:229: If reverse is True, the method is called on the 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
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.

Done.

https://codereview.appspot.com/14433049/diff/1/hooks/backend.py#newcode314
hooks/backend.py:314: """Execute the charm stopping steps."""
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
hooks/backend.py:318: """Execute the charm removal steps."""
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/shelltoolbox.py
File hooks/shelltoolbox.py (right):

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.

Good idea, done.

[frankban, 2013-10-08 08:00:14]

Hi Gary and Rick, thanks for your reviews!