Code review comment for lp:~menno.smits/juju-core/1194481-relation_name_in_status.0

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

I know we need to keep the "(status: info)" bit for old clients, but I
thought we were going to do it properly as well? Otherwise looks good.

https://codereview.appspot.com/93480044/diff/20001/state/api/client.go
File state/api/client.go (right):

https://codereview.appspot.com/93480044/diff/20001/state/api/client.go#newcode114
state/api/client.go:114: Relations map[string]RelationStatus
so we're turning int ids into strings to satisfy json? fair enough, I
guess, but I have a bit of the same old "eww" reaction.

https://codereview.appspot.com/93480044/diff/20001/state/apiserver/client/api_test.go
File state/apiserver/client/api_test.go (right):

https://codereview.appspot.com/93480044/diff/20001/state/apiserver/client/api_test.go#newcode211
state/apiserver/client/api_test.go:211: AgentStateData:
params.StatusData{"foo": "bar"},
Were we going to add the components of the "(error: blam)" stuff into
StatusData (or something similar)?

The contents of StatusData are technically arbitrary, but they're
actually pretty well-defined. We could tighten them up to prevent key
collisions with whatever we pick; or we could put the arbitrary data
into its own dict?

https://codereview.appspot.com/93480044/diff/20001/state/apiserver/client/status.go
File state/apiserver/client/status.go (right):

https://codereview.appspot.com/93480044/diff/20001/state/apiserver/client/status.go#newcode536
state/apiserver/client/status.go:536: subordSet.Add(ep.ServiceName)
I was thinking it might be simpler to keep all the relation processing
together, but then I realised that, no, we need to have all the
relations and all the services in memory together before we can do it
smartly.

https://codereview.appspot.com/93480044/

« Back to merge proposal