state: wait for initialization before returning from Open.

state: wait for initialization before returning from Open.

Also remove Initialize from the set of State methods.

https://code.launchpad.net/~rogpeppe/juju/go-rearrange-state-initialization/+merge/98161

(do not edit description out of merge proposal)

[rog, 2012-03-19 08:34:24]

Please take a look.

[rog, 2012-03-19 10:17:00]

Please take a look.

[fwereade, 2012-03-19 10:52:06]

https://codereview.appspot.com/5853048/diff/4001/state/state.go
File state/state.go (right):

https://codereview.appspot.com/5853048/diff/4001/state/state.go#newcode253
state/state.go:253: case <-time.After(3 * time.Minute):
As discussed on IRC, I don't think this is right; state.Open and state.WaitOpen
may be more appropriate.

[rog, 2012-03-19 10:57:43]

Please take a look.

[rog, 2012-03-19 11:18:25]

Please take a look.

[TheMue, 2012-03-19 13:16:53]

LGTM, only one small note.

https://codereview.appspot.com/5853048/diff/6002/state/state_test.go
File state/state_test.go (right):

https://codereview.appspot.com/5853048/diff/6002/state/state_test.go#newcode112
state/state_test.go:112: time.Sleep(0.2e9)
Nothing big, but using the constants from package time improves readability: 200
* time.Millisecond.

[niemeyer, 2012-03-19 13:27:05]

https://codereview.appspot.com/5853048/diff/6002/state/open.go
File state/open.go (right):

https://codereview.appspot.com/5853048/diff/6002/state/open.go#newcode28
state/open.go:28: func Open(info *Info) (*State, error) {
In which circumstances do we want to call Open without the state being
initialized?  Can't we simply have an Open function that waits for a pre-defined
amount of time?

[rog, 2012-03-19 13:48:07]

Please take a look.

[niemeyer, 2012-03-19 14:04:32]

LGTM, much better. Thanks.

https://codereview.appspot.com/5853048/diff/4016/state/open.go
File state/open.go (right):

https://codereview.appspot.com/5853048/diff/4016/state/open.go#newcode39
state/open.go:39: // the environment connected to.
The "ZooKeeper node" term is used to refer to the filesystem entries rather than
the servers. I suggest something like this:

// Initialize sets up an initial empty state in ZooKeeper
// and returns it. This needs to be preformed only once
// for a given cluster of ZooKeeper servers.

https://codereview.appspot.com/5853048/diff/4016/state/state.go
File state/state.go (right):

https://codereview.appspot.com/5853048/diff/4016/state/state.go#newcode240
state/state.go:240: func (s *State) waitForInitialization() error {
Shouldn't these be moved to open.go? They look strongly related.

[rog, 2012-03-19 14:26:10]

Please take a look.

https://codereview.appspot.com/5853048/diff/4016/state/open.go
File state/open.go (right):

https://codereview.appspot.com/5853048/diff/4016/state/open.go#newcode39
state/open.go:39: // the environment connected to.
On 2012/03/19 14:04:32, niemeyer wrote:
> The "ZooKeeper node" term is used to refer to the filesystem entries rather
than
> the servers. I suggest something like this:
> 
> // Initialize sets up an initial empty state in ZooKeeper
> // and returns it. This needs to be preformed only once
> // for a given cluster of ZooKeeper servers.

Done.

[rog, 2012-03-19 14:27:14]

PTAL

https://codereview.appspot.com/5853048/diff/4016/state/state.go
File state/state.go (right):

https://codereview.appspot.com/5853048/diff/4016/state/state.go#newcode240
state/state.go:240: func (s *State) waitForInitialization() error {
On 2012/03/19 14:04:32, niemeyer wrote:
> Shouldn't these be moved to open.go? They look strongly related.

Done.

[niemeyer, 2012-03-19 14:28:36]

LGTM

[rog, 2012-03-19 14:33:12]

*** Submitted:

state: wait for initialization before returning from Open.

Also remove Initialize from the set of State methods.

R=fwereade, TheMue, niemeyer
CC=
https://codereview.appspot.com/5853048