Merge lp://staging/~gary/charms/precise/juju-gui/bug1117896 into lp://staging/~juju-gui/charms/precise/juju-gui/trunk

Proposed by Gary Poster
Status: Merged
Merged at revision: 34
Proposed branch: lp://staging/~gary/charms/precise/juju-gui/bug1117896
Merge into: lp://staging/~juju-gui/charms/precise/juju-gui/trunk
Diff against target: 266 lines (+63/-11)
8 files modified
config.yaml (+6/-0)
config/config.js.template (+1/-1)
config/haproxy.cfg.template (+1/-1)
hooks/config-changed (+6/-2)
hooks/install (+5/-3)
hooks/start (+1/-1)
hooks/utils.py (+42/-2)
revision (+1/-1)
To merge this branch: bzr merge lp://staging/~gary/charms/precise/juju-gui/bug1117896
Reviewer Review Type Date Requested Status
Juju GUI Hackers Pending
Review via email: mp+152973@code.staging.launchpad.net

Commit message

fix charmhelpers incompatibility; add an "insecure" mode for the charm to support CI; reduce the time to start the charm with the gui release by about 2.5 minutes

Description of the change

This is work from bcsaller and hatch that I joined in on at the end to fix bug 1117896 as well.

Their code adds an "insecure" mode for the charm that we need to run the charm in CI against IE, which doesn't like our self-signed cert.

It also fixes the charmhelpers incompatibility.

My code makes getting the stage and build dependencies dependent on whether we need them. In my one test, getting the stage dependencies took 30 seconds on ec2, and getting the build dependencies (pre-make) took 110 seconds, so by not getting these dependencies we shave another 2 minutes and 20 seconds from the default installation. (For comparison, getting the tarball saves about 6 minutes against doing a full build, looking at the makefile alone.)

To post a comment you must log in.
Revision history for this message
Gary Poster (gary) wrote :

I'll make these changes, but these are pertinent to the work by bcsaller and hatch. :-)

In config.yaml, I plan to change "Do not set this property unless you are clear of the risks." to "This should never be used for production, and is only present to ease testing."

Per hatch's comment, I'll remove the print statements.

I want to add a comment about how that INSECURE comment is working in the haproxy config.

Revision history for this message
Gary Poster (gary) wrote :

Unfortunately this is backwards incompatible. This is because GUI releases prior to 0.2.2 needed socket_protocol, so trying to use the charm with any earlier release breaks. I discovered this because the charm tests broke. It used 0.2.1.

I updated the tests to use 0.2.2, but that's not the right solution. We should make this backwards compatible if possible, at the very least when we notice that we are breaking something.

My proposal is that, in the GUI, we support a new socket value in the config: ignore_socket_url. If socket_url is set *or if ignore_socket_url is true* then we ignore socket_url, and build the address dynamically. Then the charm will set ignore_socket_url to True.

Alternatively, we could have the GUI look for the version number. I like my proposal better.

When we change the GUI, I would like to also document the new config options in the file as comments.

Once we change the GUI trunk, we should make another release, and then land this charm branch.

Thoughts?

Revision history for this message
Jeff Pihach (hatch) wrote :

Yes I agree with your proposal.

I would take a slightly different approach - I would add a config option to allow the admin to specify the socket url at the charm level and have that reflected down to the GUI. Then in the GUI it would be pretty simple to check to see if the socket_url is specified, and if not, build it from the supplied components.

Revision history for this message
Gary Poster (gary) wrote :

Thank you for the reply!

To make sure I understand your idea, you would say to users something like, "if you are using an older version of the GUI, set the socket URL on the charm so the GUI doesn't break." Is that right? If so, my concern is that the breakage is silent (the GUI just hangs while it fails to connect), and we don't have an opportunity to advise them what to do…and we can't rely on users to read deployment instructions IMO.

The advantage of your proposal is that it doesn't require any changes to the GUI. It already works as you describe.

I will think about the idea some more though. Thanks again! Talk to you tomorrow.

Gary

On Mar 12, 2013, at 9:48 PM, Jeff Pihach <email address hidden> wrote:

> Yes I agree with your proposal.
>
> I would take a slightly different approach - I would add a config option to allow the admin to specify the socket url at the charm level and have that reflected down to the GUI. Then in the GUI it would be pretty simple to check to see if the socket_url is specified, and if not, build it from the supplied components.
> --
> https://code.launchpad.net/~gary/charms/precise/juju-gui/bug1117896/+merge/152973
> You are the owner of lp:~gary/charms/precise/juju-gui/bug1117896.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: