Merge lp://staging/~gary/charms/precise/juju-gui/update-for-jujucharms into lp://staging/~juju-gui/charms/precise/juju-gui/trunk

Please take a look.

Reviewers: mp+161298_code.launchpad.net,

Message:
Please take a look.

Description:
Fix charm for jujucharms.com use

- Merge work from mews to support exec.d directory and relative file
paths. Add tests.
- Merge work from benji to support charmworld option needed by upcoming
gui changes. Make a small fix.
- Fix deploy tests.

Tests are still fragile, but I got them all to pass with the current
branch at one time, and verified manually that the various steps still
work.

https://code.launchpad.net/~gary/charms/precise/juju-gui/update-for-jujucharms/+merge/161298

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/8999043/

Affected files:
   M .bzrignore
   A [revision details]
   M config.yaml
   M config/config.js.template
   M hooks/backend.py
   M hooks/install
   M hooks/utils.py
   A hooks/web-relation-joined
   M revision
   M tests/deploy.test
   M tests/test_utils.py

LGTM, thanks for working through it.

https://codereview.appspot.com/8999043/

*** Submitted:

Fix charm for jujucharms.com use

- Merge work from mews to support exec.d directory and relative file
paths. Add tests.
- Merge work from benji to support charmworld option needed by upcoming
gui changes. Make a small fix.
- Fix deploy tests.

Tests are still fragile, but I got them all to pass with the current
branch at one time, and verified manually that the various steps still
work.

R=rharding, benji
CC=
https://codereview.appspot.com/8999043

https://codereview.appspot.com/8999043/diff/1/hooks/install
File hooks/install (right):

https://codereview.appspot.com/8999043/diff/1/hooks/install#newcode46
hooks/install:46: for module in os.listdir('exec.d'):
On 2013/04/27 20:29:30, benji wrote:
> Usually ".d" files are to be run in lexicographic order, so throwing a
sort()
> around the listdir would be good.

Good idea in the abstract, and I changed it.

FWIW, this code is provided by IS for their use, and I don't like the
pattern and at least mew has agreed with me, so I'm not super excited
about improving it. Your ideas were good, though, so I grudgingly did
them anyway. :-)

https://codereview.appspot.com/8999043/diff/1/hooks/install#newcode49
hooks/install:49: except OSError:
On 2013/04/27 20:29:30, benji wrote:
> It would be safer if we knew which OSErrors in particular we should
ignore. It
> would also be nice to know why we are ignoring them (i.e., a comment).

OK, good point...I did it, because I agree with you, though I had to
guess at their intent. Hopefully I got it right. :-/

https://codereview.appspot.com/8999043/diff/1/hooks/utils.py
File hooks/utils.py (right):

https://codereview.appspot.com/8999043/diff/1/hooks/utils.py#newcode195
hooks/utils.py:195: """Log when a hook starts and stops its execution.
On 2013/04/27 20:29:30, benji wrote:
> Yay! I hate the "an" before non-silent "h" thing.

> I know, I know; I'm weird.

heh

https://codereview.appspot.com/8999043/diff/1/hooks/utils.py#newcode228
hooks/utils.py:228: if source[0] != '/':
On 2013/04/27 20:29:30, benji wrote:
> I don't really prefer it, but just in case you do:

> if not source.startswith('/'):

Yeah, I do, thanks. Changed.

https://codereview.appspot.com/8999043/diff/1/hooks/web-relation-joined
File hooks/web-relation-joined (right):

https://codereview.appspot.com/8999043/diff/1/hooks/web-relation-joined#newcode9
hooks/web-relation-joined:9:
On 2013/04/27 20:29:30, benji wrote:
> Unnecessary newline.

I was doing as the Romans do, but eh, I don't think the Romans will
notice. Deleted.

https://codereview.appspot.com/8999043/diff/1/hooks/web-relation-joined#newcode11
hooks/web-relation-joined:11: log_hook,
On 2013/04/27 20:29:30, benji wrote:
> You can use a one-liner here.

Again, I was following the local patterns in the charm, but eh, I agree.
  Changed.

https://codereview.appspot.com/8999043/diff/1/hooks/web-relation-joined#newcode18
hooks/web-relation-joined:18: hostname = socket.gethostname()
On 2013/04/27 20:29:30, benji wrote:
> I'm pretty sure lines 16-18 are equivalent to

> hostname = socket.getfqdn()

I stared at this for a bit. I think you are probably right. What I had
here is what the haproxy or apache2 charms are...

thank you both!

https://codereview.appspot.com/8999043/