uniter: subordinate units handle relation death

uniter: subordinate units handle relation death



https://code.launchpad.net/~fwereade/juju-core/omg-destroy-subordinates/+merge/141897

Requires: https://code.launchpad.net/~fwereade/juju-core/omg-subordinates/+merge/141737

(do not edit description out of merge proposal)

[fwereade, 2013-01-04 11:34:05]

Please take a look.

[rog, 2013-01-08 16:17:50]

looks good, with some comments below.

https://codereview.appspot.com/7030061/diff/1/worker/uniter/uniter.go
File worker/uniter/uniter.go (right):

https://codereview.appspot.com/7030061/diff/1/worker/uniter/uniter.go#newcode396
worker/uniter/uniter.go:396: if !u.unit.IsPrincipal() {
if u.unit.IsPrincipal {
    return added, nil
}

to keep the indentation level down?

https://codereview.appspot.com/7030061/diff/1/worker/uniter/uniter_test.go
File worker/uniter/uniter_test.go (right):

https://codereview.appspot.com/7030061/diff/1/worker/uniter/uniter_test.go#ne...
worker/uniter/uniter_test.go:712: buf := &bytes.Buffer{}
var buf bytes.Buffer ?

https://codereview.appspot.com/7030061/diff/1/worker/uniter/uniter_test.go#ne...
worker/uniter/uniter_test.go:716: hasher := sha256.New()
can we merge this code with the stuff in createCharm, as we discussed?

[TheMue, 2013-01-08 16:25:58]

LGTM, only supporting Rogers hint.

https://codereview.appspot.com/7030061/diff/1/worker/uniter/uniter.go
File worker/uniter/uniter.go (right):

https://codereview.appspot.com/7030061/diff/1/worker/uniter/uniter.go#newcode396
worker/uniter/uniter.go:396: if !u.unit.IsPrincipal() {
On 2013/01/08 16:17:50, rog wrote:
> to keep the indentation level down?

+1 for shortcut.

[fwereade, 2013-01-08 17:24:25]

Please take a look.

https://codereview.appspot.com/7030061/diff/1/worker/uniter/uniter.go
File worker/uniter/uniter.go (right):

https://codereview.appspot.com/7030061/diff/1/worker/uniter/uniter.go#newcode396
worker/uniter/uniter.go:396: if !u.unit.IsPrincipal() {
On 2013/01/08 16:25:58, TheMue wrote:
> On 2013/01/08 16:17:50, rog wrote:
> > to keep the indentation level down?
> 
> +1 for shortcut.

Done.

https://codereview.appspot.com/7030061/diff/1/worker/uniter/uniter_test.go
File worker/uniter/uniter_test.go (right):

https://codereview.appspot.com/7030061/diff/1/worker/uniter/uniter_test.go#ne...
worker/uniter/uniter_test.go:712: buf := &bytes.Buffer{}
On 2013/01/08 16:17:50, rog wrote:
> var buf bytes.Buffer ?

/shrug. Done.

https://codereview.appspot.com/7030061/diff/1/worker/uniter/uniter_test.go#ne...
worker/uniter/uniter_test.go:716: hasher := sha256.New()
On 2013/01/08 16:17:50, rog wrote:
> can we merge this code with the stuff in createCharm, as we discussed?

Hell, forgot to repropose -- sorry. Done.

[rog, 2013-01-08 17:46:41]

LGTM

https://codereview.appspot.com/7030061/diff/7001/worker/uniter/uniter_test.go
File worker/uniter/uniter_test.go (right):

https://codereview.appspot.com/7030061/diff/7001/worker/uniter/uniter_test.go...
worker/uniter/uniter_test.go:716: step(c, ctx, addCharm{dir, curl})
much nicer, thanks!

https://codereview.appspot.com/7030061/diff/7001/worker/uniter/uniter_test.go...
worker/uniter/uniter_test.go:816: coretesting.Server.ResponseMap(1, ctx.charms)
serving the charm for exactly one request seems to me to be a little fragile
(what if the implementation makes some other request first?), but we'll leave it
be for now, as it's established.

[fwereade, 2013-01-08 22:59:54]

*** Submitted:

uniter: subordinate units handle relation death

R=rog, TheMue
CC=
https://codereview.appspot.com/7030061

https://codereview.appspot.com/7030061/diff/7001/worker/uniter/uniter_test.go
File worker/uniter/uniter_test.go (right):

https://codereview.appspot.com/7030061/diff/7001/worker/uniter/uniter_test.go...
worker/uniter/uniter_test.go:816: coretesting.Server.ResponseMap(1, ctx.charms)
On 2013/01/08 17:46:42, rog wrote:
> serving the charm for exactly one request seems to me to be a little fragile
> (what if the implementation makes some other request first?), but we'll leave
it
> be for now, as it's established.

It was established in the first place to check that the uniter doesn't waste
time redownloading charms it's already got; I think that's still a sensible
thing to verify.