Code review comment for lp:~themue/juju-core/024-cleanup-watcher

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

https://codereview.appspot.com/10078043/diff/1/state/watcher.go#newcode1244
state/watcher.go:1244: // After a unit is found to be Dead, no further
event will include it.
On 2013/06/06 16:26:44, fwereade wrote:
> Fix these please :).

Ouch, yes!

https://codereview.appspot.com/10078043/diff/1/state/watcher.go#newcode1283
state/watcher.go:1283: next := time.After(watcher.Period)
On 2013/06/06 16:26:44, fwereade wrote:
> Why is this not a... watcher? We discussed this yesterday: there's no
call
> whatsoever to make this periodic. We can just use the usual,
conventional
> mechanism of watching the st.cleanups collection, but with the
additional
> simplification that I'm happy just sending out a notification whenever
there's
> any change whatsoever to any document in there.

The usual mechanism relies on entities with TxnRevno. I wanted to use it
first, but then I've seen that the cleanupDoc doesn't uses it. Es
explicit as most of our code is I thought that this decision has not
been by accident. So I implemented a polling with the same timeframe the
standard watcher uses to recognize changes.

I surely can change it to the standard watcher adding the TxnRevno. I
only see that it leads to the sending of an event on any change. This
would later lead to events during the cleanup and so also lead to even
more Cleanup() calls. I've tried to avoid it. How would you avoid those
events during/after a Cleanup() run?

> I really tried to be as explicit as possible; what do you think is the
source of
> the communication problems here?

That's a different topic and we tomorrow should take some time to have a
talk via Hangout.

https://codereview.appspot.com/10078043/