Code review comment for lp:~rogpeppe/pyjuju/go-state-units-under-service

Revision history for this message
William Reade (fwereade) wrote :

LGTM modulo below.

https://codereview.appspot.com/6247066/diff/7001/state/service.go
File state/service.go (right):

https://codereview.appspot.com/6247066/diff/7001/state/service.go#newcode120
state/service.go:120: return zkRemoveTree(s.st.zk, unit.zkPath())
Not relevant to this CL... but do we actually want to do this? The unit
agent will surely still be running; I'm not sure it'll handle this all
that well...

https://codereview.appspot.com/6247066/diff/7001/state/service.go#newcode228
state/service.go:228: return s.zkPath() + "/config"
I strongly approve of all the changes like this one :).

https://codereview.appspot.com/6247066/diff/7001/state/topology.go
File state/topology.go (left):

https://codereview.appspot.com/6247066/diff/7001/state/topology.go#oldcode283
state/topology.go:283: // sequence number will be increased
monotonically for each service.
Update comment

https://codereview.appspot.com/6247066/diff/7001/state/unit.go
File state/unit.go (right):

https://codereview.appspot.com/6247066/diff/7001/state/unit.go#newcode77
state/unit.go:77: func mkUnitKey(serviceKey string, unitId int) string {
What does "mk" signify?

https://codereview.appspot.com/6247066/diff/7001/state/unit.go#newcode86
state/unit.go:86: return "", fmt.Errorf("invalid unit key %q (1) %s",
unitKey, debug.Callers(0, 10))
Is this the proposed implementation or an oversight?

https://codereview.appspot.com/6247066/

« Back to merge proposal