Fix charm for jujucharms.com use

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)

[gary.poster, 2013-04-27 19:14:44]

Please take a look.

[rharding, 2013-04-27 19:32:29]

LGTM, thanks for working through it.

[benji, 2013-04-27 20:29:30]

This branch looks good.  Don't be fooled by the number of comments.  I was
apparently either very picky or very enlightening.  You be the judge.

LGTM-ly y'rs

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'):
Usually ".d" files are to be run in lexicographic order, so throwing a sort()
around the listdir would be good.

https://codereview.appspot.com/8999043/diff/1/hooks/install#newcode49
hooks/install:49: except OSError:
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).

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.
Yay!  I hate the "an" before non-silent "h" thing.


I know, I know; I'm weird.

https://codereview.appspot.com/8999043/diff/1/hooks/utils.py#newcode228
hooks/utils.py:228: if source[0] != '/':
I don't really prefer it, but just in case you do:

    if not source.startswith('/'):

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: 
Unnecessary newline.

https://codereview.appspot.com/8999043/diff/1/hooks/web-relation-joined#newco...
hooks/web-relation-joined:11: log_hook,
You can use a one-liner here.

https://codereview.appspot.com/8999043/diff/1/hooks/web-relation-joined#newco...
hooks/web-relation-joined:18: hostname = socket.gethostname()
I'm pretty sure lines 16-18 are equivalent to

hostname = socket.getfqdn()

https://codereview.appspot.com/8999043/diff/1/tests/test_utils.py
File tests/test_utils.py (right):

https://codereview.appspot.com/8999043/diff/1/tests/test_utils.py#newcode406
tests/test_utils.py:406: self.assertTupleEqual(expected,
parse_source('url:http://example.com/gui'))
I think this line is over 79 characters.

https://codereview.appspot.com/8999043/diff/1/tests/test_utils.py#newcode414
tests/test_utils.py:414: self.assertTupleEqual(expected,
parse_source('url:foo/bar'))
Recent versions of unittest automatically pick the assert function that matches
the datatype so you can just do assertEqual and it will call assertTupleEqual
for you.

[gary.poster, 2013-04-28 01:48:15]

*** 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#newco...
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#newco...
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 doing, I forget.  Under the time
pressure, it is arguably better to go with the proven code.  However,  decided
to go with the simpler version you proposed.  If it doesn't work, we'll find out
why and add a comment about it, unlike the other charm I saw.

https://codereview.appspot.com/8999043/diff/1/tests/test_utils.py
File tests/test_utils.py (right):

https://codereview.appspot.com/8999043/diff/1/tests/test_utils.py#newcode406
tests/test_utils.py:406: self.assertTupleEqual(expected,
parse_source('url:http://example.com/gui'))
On 2013/04/27 20:29:30, benji wrote:
> I think this line is over 79 characters.

Done.

https://codereview.appspot.com/8999043/diff/1/tests/test_utils.py#newcode414
tests/test_utils.py:414: self.assertTupleEqual(expected,
parse_source('url:foo/bar'))
On 2013/04/27 20:29:30, benji wrote:
> Recent versions of unittest automatically pick the assert function that
matches
> the datatype so you can just do assertEqual and it will call assertTupleEqual
> for you.

That sounds cool.  For now, I'm sticking with these here local Romans and their
assertTupleEqual calls, as found in all proceeding tests in this case.  We can
switch out later in another branch, as desired.

[gary.poster, 2013-04-28 01:50:45]

thank you both!