Fwd: [Bug 1103035] Re: Charm needed: Juju GUI

[Gary Poster]

On 01/22/2013 03:47 PM, Gary Poster wrote:
> Hi all.  Here are things to address in the charm.
> 
> I certainly agree that what we have built is pretty darn nice. :-) Thank
> you!
> 
> Gary
> 
> 
> -------- Original Message --------
> Subject: [Bug 1103035] Re: Charm needed: Juju GUI
> Date: Tue, 22 Jan 2013 21:32:45 -0000
> From: Robert Ayres <robert.ayres at canonical.com>
> Reply-To: Bug 1103035 <1103035 at bugs.launchpad.net>
> To: gary.poster at canonical.com
> 
> Thanks for your submission.  A very nice charm :)
> 
> I've tested on EC2 and LXC.
> 
> Please see bugs/comments below.

My thoughts on these, fwiw.

> 
> Bugs:
> *'curl' needs to be added as an apt-get install dependency if you use
> the charm under LXC.

+1 and yay him!  He did a great job on this review.

> 
> *If you use the 'user', 'password' config options then these can be
> obtained simply by accessing the URL - https://xxx/juju-
> ui/assets/config.js .

I am inclined to think that this is merely a warning that we add to
those configuration values.  Alternatively, do we have a use case for
this other than improv?  If we don't, maybe we should remove these
options as dangerous and only set the admin/admin authentication with
the "staging" module?

> 
> Comments:
> *metadata.yaml, for consistency I'd move the maintainer line to below
> 'summary'.

might as well.

> 
> *I'd mention in the README that you may need to add a https certificate
> exception for self-signed certificates for port 8080 ('juju-api-port').
> I found I couldn't get the UI to appear unless I first accessed on port
> 8080 and accepted the self-signed certificate.

good idea and nice thorough diagnosis

> 
> *I'd anonymise the EC2 address in the README.  You never know, this
> charm might generate accidental traffic for a real site :)

heh, sure.

> 
> Please fix the bugs and consider the comments, then reopen for a second
> review.
> 
> ** Changed in: charms
>        Status: New => Incomplete
>