state: improve machine watcher

state: improve machine watcher

This fixes it so that initial events are sent, and it also
drops the result out of the channel. This is an experiment,
and the result may come back if necessary.

https://code.launchpad.net/~niemeyer/juju-core/better-machine-watcher/+merge/126301

(do not edit description out of merge proposal)

[niemeyer, 2012-09-25 17:36:18]

Please take a look.

[aram, 2012-09-25 17:41:41]

LGTM

https://codereview.appspot.com/6564049/diff/1/state/machine_test.go
File state/machine_test.go (right):

https://codereview.appspot.com/6564049/diff/1/state/machine_test.go#newcode293
state/machine_test.go:293: case <-time.After(50 * time.Millisecond):
I don't like we use a different value in different places. We should have a
symbol for this and use it everywhere. That being said, a smaller value is
great.

[niemeyer, 2012-09-25 17:45:48]

https://codereview.appspot.com/6564049/diff/1/state/machine_test.go
File state/machine_test.go (right):

https://codereview.appspot.com/6564049/diff/1/state/machine_test.go#newcode293
state/machine_test.go:293: case <-time.After(50 * time.Millisecond):
On 2012/09/25 17:41:41, aram wrote:
> I don't like we use a different value in different places. We should have a
> symbol for this and use it everywhere. That being said, a smaller value is
> great.

I've been changing everywhere I see to 50ms on the happy case and 5 seconds on
the sad one.

[aram, 2012-09-25 17:49:27]

> I've been changing everywhere I see to 50ms on the happy case and 5 seconds on
> the sad one.

white:state$ g '50 \* time\.Millisecond' | grep test | wc -l
2
white:state$ g '100 \* time\.Millisecond' | grep test | wc -l
12

[niemeyer, 2012-09-25 17:53:06]

On 2012/09/25 17:49:27, aram wrote:
> > I've been changing everywhere I see to 50ms on the happy case and 5 seconds
on
> > the sad one.
> 
> white:state$ g '50 \* time\.Millisecond' | grep test | wc -l
> 2
> white:state$ g '100 \* time\.Millisecond' | grep test | wc -l
> 12

Sorry, let me restate for clarity:

If you touch code with timeouts, make sure the happy case is 50ms, and the sad
one is 5 seconds.

Thank you.

[rog, 2012-09-25 18:00:25]

LGTM with a AddMachine/Machine consistency check as discussed on line.

https://codereview.appspot.com/6564049/diff/1/state/machine_test.go
File state/machine_test.go (right):

https://codereview.appspot.com/6564049/diff/1/state/machine_test.go#newcode293
state/machine_test.go:293: case <-time.After(50 * time.Millisecond):
On 2012/09/25 17:45:48, niemeyer wrote:
> On 2012/09/25 17:41:41, aram wrote:
> > I don't like we use a different value in different places. We should have a
> > symbol for this and use it everywhere. That being said, a smaller value is
> > great.
> 
> I've been changing everywhere I see to 50ms on the happy case and 5 seconds on
> the sad one.

const (
    sadTime = 5 * time.Second
    happyTime = 50 * time.Millisecond
)

?

[niemeyer, 2012-09-25 18:33:56]

Please take a look.

[niemeyer, 2012-09-25 18:38:07]

*** Submitted:

state: improve machine watcher

This fixes it so that initial events are sent, and it also
drops the result out of the channel. This is an experiment,
and the result may come back if necessary.

R=aram, rog
CC=
https://codereview.appspot.com/6564049