On 30.11.2012 01:18, Richard Su wrote:
On 11/29/2012 08:33 AM, Jiří Stránský wrote:
> Hi,
>
> I'd like to change some stuff in the deployments API draft and I'd like some
ACKs and/or comments on the numbered issues below. I know this API already went around
once in a RFC and sort of passed, but I think we missed things in there and didn't
decide others. I'd like to get the implementation right so that we don't have to
change it later :) The current draft is on wiki, the rest of this mail refers to this
draft [1].
>
> 1) <owner_id>
>
> *I wouldn't show owner information at all for now.* In an ideal case, this would
be a proper link to a "user" resource, but we don't have API to access user
information now. I think there's not much point in showing just an ID. We can add the
<user> link when we have the "user" resource implemented.
>
Good point, I think you're pointing out the fact that we do need to implement the
user resource. As a first pass perhaps we can make it read only with basic information
like name, email, but not password. We can leave owner_id out for now until that resource
has been implemented.
Hmm... not keen on this. I'm more in favour of "if you know you'll have to
change it later, don't put it there" when it comes to APIs. Adding stuff is ok
for backwards compatibility, but changing/removing is not (I think both Deltacloud and
CIMI follow this principle). With showing just <owner_id>, we don't give the
client meaningful information, but we're commiting ourselves to having to break the
backward compatibility later on (when we change it into <user>). The implementation
of <owner_id> is now just a one line of code + one line of spec I think, but I'd
like to leave it out anyway because I think it would cause more trouble than benefit.
> 2) <created_at>, <updated_at>
>
> *I wouldn't add these (yet), too.* I think our APIs, both Conductor and Tim,
don't show created_at/updated_at information, even though it is available in the
models. We could use <updated_at> with PUT requests to avoid concurrency issues, but
I'd rather see that as a project-wide decision first, not just implement it for a few
resources. So I want to leave these out to keep the APIs consistent. If we decide we want
this information in APIs, we can add it across all relevant resources.
I think we do need to include these fields in the APIs. Perhaps modified_by should be
added too for auditing purposes.
Got your point. Regarding implementation I'll wait for Aeolus-wide decision (mainly
interested what Martyn will have to say from Tim perspective). It wouldn't be cool if
we implement it and Tim doesn't :) The implementation is pretty straightforward, just
don't want to remove or change it later, from the reasons I wrote in the paragraph
above.
>
> 3) <global_uptime>5 minutes</global uptime>
>
> *I'll implement a standard way to serialize duration information - from XML
Schema[2].* E.g. 5 minutes would serialize as "P5M". CIMI uses it as well.
Sounds good. We should point to such documentation so people know what the annotations
mean.
Ok :)
>
> 4) <deployable-xml>
>
> *Should we wrap the deployable template into CDATA or not?* The benefit is that the
inner XML will not be parsed by the client when parsing the API response, so an error in
the template XML can't break the whole API response. And it is semantically cleaner,
because the template XML won't be part of the API response XML tree, but will be
treated as data, which it is. (Think if we provided JSON API, then the template XML would
be treated as a data too and not converted into a corresponding JSON structure, I'd
say.) The drawback is that our deployable then can't have a CDATA in it, because CDATA
nesting is not allowed. For deployable templates, this might not be a problem right now,
but I wonder about the future. E.g. for image templates it would be a problem [3]. So
I'm sort of on the fence here, maybe a bit in favour of not wrapping it into CDATA.
(Btw Tim solves this by having image template as a separate resource, but I'm not sure
we should go this way for deployables as!
we!
> ll.)
I would vote for a separate resource. There is xml embedded in the instance resource too,
and this sounds like the cleanest way around it.
To brainstorm further - if we make it a separate resource, I wonder if we should make it
totally separate, like Tim has it (template can be created independently of images). Say:
/api/deployabletemplates/:id
Then deployables and deployments DB records would link to these templates, and templates
would have to become read-only (create, but never update), like in Tim.
This would force us to do significant changes in the DB model as well (and I wonder if
we'd want/need to change UI workflow anyhow), because currently the template is
embedded as an attribute into deployable and deployment. This solution would be probably
very time consuming to implement, but should be possible.
Or, if we should make it as a simple nested subresource. Say:
/api/deployables/:deployable_id/template
/api/deployment/:deployment_id/template
This approaich is imho possible without performing a major surgery on Conductor and solves
the problem of API representation. The first approach on the other hand seems a bit
cleaner by design.
I cannot decide right now which one I'd prefer more :)
>
> 5) <history>
>
> *I think this should be modelled as a separate resource/collection*, as the number of
events associated with a deployment can grow limitlessly over time. CIMI has this
separated too in a form of EventLog resource, though I didn't get the time to read
this in detail yet.
>
Initially I wasn't sure if it needed to be modelled as a separate resource. But that
is looking like the right approach.
Ok.
Thanks for taking the time to discuss this, Richard :)
J.