Code review comment for lp:~menno.smits/juju-core/1194481-relation_name_in_status.0
Revision history for this message
| William Reade (fwereade) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
0e1f616
(Get the code!)
LGTM with relation ids in the array (assuming followups as discussed).
https:/
File state/api/client.go (right):
https:/
state/api/
On 2014/05/26 21:47:43, menn0 wrote:
> On 2014/05/26 06:45:34, fwereade wrote:
> > 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.
> I discovered the hard way what happens if you try to serialise
map[int] over the
> API: seemingly hung tests (perhaps due to API retries?)
> I didn't like converting the relation ids to strings much either. I
was
> considering making Relations a []RelationStatus instead and including
Id in
> RelationStatus. That way the client may end up generating a map itself
which is
> bit crappy but hey. Would that be preferable?
Hmm... yeah, I think it probably would. I also find myself wondering
whether we should actually be representing the endpoint data as well,
lest clients have to mess around parsing keys? Don't let this block you,
it's addressable in a followup, but take a quick look at the AddRelation
API: what we chose there *may* have bearing on what we should expose
here.
https:/
File state/apiserver
https:/
state/apiserver
params.
On 2014/05/26 21:47:43, menn0 wrote:
> On 2014/05/26 06:45:34, fwereade wrote:
> > Were we going to add the components of the "(error: blam)" stuff
into
> StatusData
> > (or something similar)?
> Yes that's coming (in a separate branch). This branch is just about
getting the
> supporting infrastructure in place.
Great, sorry for the confusion, I fear you will have to get used to me
asking stupid questions to which I should already know the answers :).
> The test data here isn't realistic. This test is just checking that
StatusData
> is making it where it should the status response.
> > 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?
> Agreed that we should be careful about what we put in StatusData. How
about
> namespacing the keys along the lines of "category.item" to help avoid
> collisions?
Depends what we mean by StatusData -- the stuff that gets set by the
agent, or the stuff we return over status? For the latter, I think it
may be better to separate the explicitly-
also-relevant-
https:/