mstate: add config node

mstate: add config node



https://code.launchpad.net/~aramh/juju-core/53-mstate-confignode-basic/+merge/120197

Requires: https://code.launchpad.net/~aramh/juju-core/52-mstate-convert-to-testing/+merge/120196

(do not edit description out of merge proposal)

[aram, 2012-08-17 17:14:14]

Please take a look.

[niemeyer, 2012-08-20 14:18:19]

This looks great. There are just a couple of details we should talk about in
terms of how to port the concepts:

https://codereview.appspot.com/6460105/diff/2001/mstate/confignode.go
File mstate/confignode.go (right):

https://codereview.appspot.com/6460105/diff/2001/mstate/confignode.go#newcode53
mstate/confignode.go:53: path string
Is a "path" the right abstraction here?

https://codereview.appspot.com/6460105/diff/2001/mstate/confignode.go#newcode166
mstate/confignode.go:166: {"$inc", bson.D{{"version", 1}}},
Why do we need this here?

[aram, 2012-08-20 14:24:30]

https://codereview.appspot.com/6460105/diff/2001/mstate/confignode.go
File mstate/confignode.go (right):

https://codereview.appspot.com/6460105/diff/2001/mstate/confignode.go#newcode53
mstate/confignode.go:53: path string
On 2012/08/20 14:18:20, niemeyer wrote:
> Is a "path" the right abstraction here?

No. I just replicated what was in state. Probably we should rename to key or
something.

https://codereview.appspot.com/6460105/diff/2001/mstate/confignode.go#newcode166
mstate/confignode.go:166: {"$inc", bson.D{{"version", 1}}},
On 2012/08/20 14:18:20, niemeyer wrote:
> Why do we need this here?

Initially this was missing as it's not exportet in the API, but there's a
test in state (TestMultipleWritesAreStable) that checks for it (confignode
tests in state are internal tests) to validate some important behavioral
logic, and I figured it might be a good thing to also have this test
in mstate.

[niemeyer, 2012-08-20 14:36:16]

Given those comments, LGTM to move forward as-is. We can polish those details in
follow ups.

[aram, 2012-08-22 18:58:06]

Please take a look.

[niemeyer, 2012-08-23 23:13:13]

LGTM

https://codereview.appspot.com/6460105/diff/8001/mstate/confignode.go
File mstate/confignode.go (right):

https://codereview.appspot.com/6460105/diff/8001/mstate/confignode.go#newcode48
mstate/confignode.go:48: // them back onto the disk when explicitly requested
s/disk/database/

[aram, 2012-08-24 09:45:52]

*** Submitted:

mstate: add config node

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