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

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