Code review - Issue 6292044: Any session event on a watcher now fires and closes the watch.https://codereview.appspot.com/2012-06-04T23:43:04+00:00rietveld
Message from unknown
2012-06-04T22:08:15+00:00niemeyerurn:md5:85176a3b99300b98117425c9e7b51b1e
Message from n13m3y3r@gmail.com
2012-06-04T22:08:26+00:00niemeyerurn:md5:cffbbe4ae883e5f6a7533d6821e3bf23
Please take a look.
Message from dave@cheney.net
2012-06-04T22:58:59+00:00dfcurn:md5:ad1a9fbe9dcb2a83b18f41bd1ee719ed
LGTM.
https://codereview.appspot.com/6292044/diff/1/zk.go
File zk.go (right):
https://codereview.appspot.com/6292044/diff/1/zk.go#newcode1049
zk.go:1049: // non-session watches as fatal.
this comment is a little confusing; does it mean
// .. but we take all session events that occur before a session is established as errors.
https://codereview.appspot.com/6292044/diff/1/zk_test.go
File zk_test.go (right):
https://codereview.appspot.com/6292044/diff/1/zk_test.go#newcode340
zk_test.go:340: case <-time.After(3e9):
nit: 3 * time.Second
you've imported time, so you might as well make use of the constant.
https://codereview.appspot.com/6292044/diff/1/zk_test.go#newcode648
zk_test.go:648: case <-time.After(3e9):
nit: ditto, see above
Message from unknown
2012-06-04T23:41:33+00:00niemeyerurn:md5:810cb3a6fcc151e9d506d97524afa88a
Message from n13m3y3r@gmail.com
2012-06-04T23:43:04+00:00niemeyerurn:md5:75221d797566fcfbb174b743cff16e9d
*** Submitted:
Any session event on a watcher now fires and closes the watch.
Also test that closing the connection closes the watch.
R=dfc
CC=
https://codereview.appspot.com/6292044
https://codereview.appspot.com/6292044/diff/1/zk.go
File zk.go (right):
https://codereview.appspot.com/6292044/diff/1/zk.go#newcode1049
zk.go:1049: // non-session watches as fatal.
On 2012/06/04 22:58:59, dfc wrote:
> this comment is a little confusing; does it mean
>
> // .. but we take all session events that occur before a session is established
> as errors.
Improved comment. Thanks for the poke.
https://codereview.appspot.com/6292044/diff/1/zk_test.go
File zk_test.go (right):
https://codereview.appspot.com/6292044/diff/1/zk_test.go#newcode340
zk_test.go:340: case <-time.After(3e9):
On 2012/06/04 22:58:59, dfc wrote:
> nit: 3 * time.Second
>
> you've imported time, so you might as well make use of the constant.
I've copy & pasted that. There are tons of uses here, historical. I woudln't like to change that right now if that's ok.