On Fri, Nov 30, 2012 at 11:43:19AM +0100, Jiří Stránský wrote: > 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.
We can include <user id="1" /> and add href attribute later when Users API will be there. This way we have user id there and we will just add new things to API, not changing existing. > > > > >>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 :) > I am for separated resources too. > > > >> > >>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. +1 > > Ok. > > Thanks for taking the time to discuss this, Richard :) > > J. -- Petr Blaho, [email protected] Software Engineer, CloudForms
