UnitAgent now runs a Uniter

UnitAgent now runs a Uniter



https://code.launchpad.net/~fwereade/juju-core/jujud-run-uniter/+merge/123550

(do not edit description out of merge proposal)

[fwereade, 2012-09-10 13:39:21]

Please take a look.

[rog, 2012-09-11 17:06:45]

On 2012/09/10 13:39:21, fwereade wrote:
> Please take a look.

LGTM

[rog, 2012-09-11 17:06:52]

https://codereview.appspot.com/6497109/diff/1/juju/testing/conn.go
File juju/testing/conn.go (right):

https://codereview.appspot.com/6497109/diff/1/juju/testing/conn.go#newcode140
juju/testing/conn.go:140: sch, err := s.Conn.PutCharm(curl, testing.Charms.Path,
false)
aha! oops.

[niemeyer, 2012-09-11 21:21:59]

LGTM!

https://codereview.appspot.com/6497109/diff/1/cmd/jujud/unit_test.go
File cmd/jujud/unit_test.go (right):

https://codereview.appspot.com/6497109/diff/1/cmd/jujud/unit_test.go#newcode86
cmd/jujud/unit_test.go:86: timeout := time.After(2000 * time.Millisecond)
I know it's way less cool, but that's 2 * time.Second. :-)

https://codereview.appspot.com/6497109/diff/1/cmd/jujud/unit_test.go#newcode91
cmd/jujud/unit_test.go:91: case <-time.After(200 * time.Millisecond):
s/200/50/

It's fine to spin slightly faster for responsiveness.

https://codereview.appspot.com/6497109/diff/1/cmd/jujud/unit_test.go#newcode99
cmd/jujud/unit_test.go:99: c.Logf("started!")
s/!/OMG!!!/

https://codereview.appspot.com/6497109/diff/1/cmd/jujud/unit_test.go#newcode108
cmd/jujud/unit_test.go:108: c.Assert(<-done, IsNil)
This, the goroutine, and the channel, seem unnecessary.

c.Assert(a.Stop, IsNil)

[fwereade, 2012-09-12 07:26:15]

*** Submitted:

UnitAgent now runs a Uniter

R=rog, niemeyer
CC=
https://codereview.appspot.com/6497109

https://codereview.appspot.com/6497109/diff/1/cmd/jujud/unit_test.go
File cmd/jujud/unit_test.go (right):

https://codereview.appspot.com/6497109/diff/1/cmd/jujud/unit_test.go#newcode86
cmd/jujud/unit_test.go:86: timeout := time.After(2000 * time.Millisecond)
On 2012/09/11 21:21:59, niemeyer wrote:
> I know it's way less cool, but that's 2 * time.Second. :-)

Bumped to 3 in line with other CL

https://codereview.appspot.com/6497109/diff/1/cmd/jujud/unit_test.go#newcode91
cmd/jujud/unit_test.go:91: case <-time.After(200 * time.Millisecond):
On 2012/09/11 21:21:59, niemeyer wrote:
> s/200/50/
> 
> It's fine to spin slightly faster for responsiveness.

I have an unsupported feeling that the harder I work to check status quickly,
the more likely it is that I'll hit the global timeout because I'm starving the
uniter of resources. I'll go with this though :).

https://codereview.appspot.com/6497109/diff/1/cmd/jujud/unit_test.go#newcode99
cmd/jujud/unit_test.go:99: c.Logf("started!")
On 2012/09/11 21:21:59, niemeyer wrote:
> s/!/OMG!!!/

:D

https://codereview.appspot.com/6497109/diff/1/cmd/jujud/unit_test.go#newcode108
cmd/jujud/unit_test.go:108: c.Assert(<-done, IsNil)
On 2012/09/11 21:21:59, niemeyer wrote:
> This, the goroutine, and the channel, seem unnecessary.
> 
> c.Assert(a.Stop, IsNil)

Run blocks, so I can't see how to make it work (and check that Run actually does
stop) without this machinery. Submitting as-is; will gladly do a trivial
followup once you've explained what I missed :).