state: add machine units watcher

state: add machine units watcher



https://code.launchpad.net/~aramh/juju-core/82-mstate-watchers-machine-units3/+merge/125455

(do not edit description out of merge proposal)

[aram, 2012-09-20 10:50:20]

Please take a look.

[niemeyer, 2012-09-20 13:24:14]

https://codereview.appspot.com/6545044/diff/1/state/watcher.go
File state/watcher.go (right):

https://codereview.appspot.com/6545044/diff/1/state/watcher.go#newcode83
state/watcher.go:83: type MachineUnitsWatcher struct {
s/Machine//

We don't have to worry about the implementation for now.

https://codereview.appspot.com/6545044/diff/1/state/watcher.go#newcode90
state/watcher.go:90: type MachineUnitsChange struct {
s/Machine//

https://codereview.appspot.com/6545044/diff/1/state/watcher.go#newcode654
state/watcher.go:654: // WatchUnits returns a watcher for observing units being
s/WatchUnits/WatchPrincipalUnits/

https://codereview.appspot.com/6545044/diff/1/state/watcher.go#newcode703
state/watcher.go:703: if err != nil {
What if someone removed it meanwhile?

https://codereview.appspot.com/6545044/diff/1/state/watcher.go#newcode728
state/watcher.go:728: err = w.st.units.Find(D{{"machineid",
w.machine.Id()}}).All(&docs)
It should use the machine here. We're subscribed to it, and that's what deltas
will be built against.

We can still use a single find, though, that looks for all the ids "$in" the
given selection of ids

[aram, 2012-09-20 14:30:34]

Please take a look.

https://codereview.appspot.com/6545044/diff/1/state/watcher.go
File state/watcher.go (right):

https://codereview.appspot.com/6545044/diff/1/state/watcher.go#newcode654
state/watcher.go:654: // WatchUnits returns a watcher for observing units being
On 2012/09/20 13:24:14, niemeyer wrote:
> s/WatchUnits/WatchPrincipalUnits/

Done.

https://codereview.appspot.com/6545044/diff/1/state/watcher.go#newcode703
state/watcher.go:703: if err != nil {
On 2012/09/20 13:24:14, niemeyer wrote:
> What if someone removed it meanwhile?

Done.

https://codereview.appspot.com/6545044/diff/1/state/watcher.go#newcode728
state/watcher.go:728: err = w.st.units.Find(D{{"machineid",
w.machine.Id()}}).All(&docs)
On 2012/09/20 13:24:14, niemeyer wrote:
> It should use the machine here. We're subscribed to it, and that's what deltas
> will be built against.
> 
> We can still use a single find, though, that looks for all the ids "$in" the
> given selection of ids

Done.

[niemeyer, 2012-09-20 15:41:14]

LGTM with the minor we talked about.

[aram, 2012-09-20 15:46:46]

*** Submitted:

state: add machine units watcher

R=niemeyer
CC=
https://codereview.appspot.com/6545044