API RFC: Information in deployable #show

Martyn Taylor mtaylor at redhat.com
Wed Dec 12 14:16:14 UTC 2012


On 12/12/2012 11:06 AM, Jiří Stránský wrote:
> On 10.12.2012 14:34, Petr Blaho wrote:
>> 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.
>>
>
> Yes this would be possible. However, there's also a question of how to 
> show relationship semantics. For the first time I think, we hit the 
> issue in API that the name of the attribute is different from the 
> resource name. The attribute in question is not called "user", it is 
> called "owner". I'd kinda like to indicate this somehow in the 
> response for two reasons:
>
> * Simply to make clear what the relationship means. It's the same 
> reason why our data model has the relationship named "owner", not 
> "user". I believe this information shouldn't get lost.
>
> * In case there are multiple users linked to a deployment (or sth 
> else) in the future, we'd *have* to indicate semantics to avoid 
> ambiguity of having two <user> links.
>
> Maybe this could be indicated by <user rel="owner" ... />. I'm not 
> sure of this and it's not a priority question right now, but imho it's 
> a reason for not putting the <user> there at all for now.
The other question here is really about whether the owner/user should be 
exposed in the API ever.  Do users need to know this information or is 
it simply an implementation detail.

Is there any value in showing the user who owns an image?  What can they 
do with this information?  When we are writing the API we should not 
blindly copy the database models.  We need think about what makes sense.
>
> [snip]
>>>>> 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. 
This CDATA concept is very strange to me.  The original reason for us 
using CDATA was because of the initial crappy implementation we had in 
the images API.  Template is a resource just like any other. We should 
parse it and validate it like any other.

>>>>> (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 !
CDATA is only relevant when we are using an XML parser.  Having this 
attribute in JSON means nothing.  Tim treats Templates just like any 
other resource.  We do not need to tell the parser to ignore it's content.
> 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.
Yes this follows the pattern we have elsewhere in the API.
>>>
>>> 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.
Are we talking about deployable templates here?  Or Oz Templates? We 
should create separate resources for both.
>>>
>>>
>>> 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.
This does not conform to what we have elsewhere in the API.  Here you 
are exposing an attribute via a URL.  We should only use URLs tha tpoint 
to resources.
>>>
>>> I cannot decide right now which one I'd prefer more :)
>>>
>>
>> I am for separated resources too.
>
> Hehe and Richard just wrote that we should embed :) Anyways, yesterday 
> I found that CDATA sections *can* be nested [1], even though it's more 
> of a trick than a designed way. So, I think we could use CDATA for 
> embedding XML after all.
>
> CDATA pros:
> * Seems to be the correct way (compared to non-CDATA embedding) when 
> we treat templates as data, not as resources
>
> CDATA cons:
> * Tim doesn't use it, Tim allows embedding templates without CDATA [2] 
> -> we would be inconsistent
> * We would force clients to use the trick [1] in their POST/PUT 
> requests if their templates contained CDATA.
>
> A problem I think of with nested resources: Even if we make it nested 
> resource for GET requests, we'd still have to support embedding in 
> POST for *deployables* (deployable has to be created with it's 
> template in the current data model). It doesn't remove embedding 
> completely.
>
> Unless I missed something important, I'm tempted to go the way of 
> simple embedding without CDATA, as Richard suggested.
>
>
+1
> Take care,
>
> J.
>
> [1] http://en.wikipedia.org/wiki/CDATA#Nesting
> [2] 
> https://github.com/aeolus-incubator/tim#create-base-image-with-template-a-single-image-version-with-a-single-target-image



More information about the aeolus-devel mailing list