[Engine-devel] Fwd: Problem in REST API handling/displaying of logical networks

[Michael Pasternak]

On 07/05/2012 01:43 PM, Mike Kolesnik wrote:
> ----- Original Message -----
>> On 05/07/12 13:23, Mike Kolesnik wrote:
>>>> On 07/05/2012 12:19 PM, Livnat Peer wrote:
>>>>>> I'll give you one scenario and I'm sure there are lot more:
>>>>>>> delete all unused networks ....
>>>>>>>
>>>>> not strong enough use case in my opinion
>>>>
>>>> i do see sense in this, and based on my experience of
>>>> closing ~5 bugs on this for SD and explaining like
>>>> ~10 times on ML to users why /api/storagedomains/xxx
>>>> doesn't have <status>, I'm sure it should be done this way
>>>> as it creates clear differentiation between root-resource
>>>> and cluster-resource (shared) status.
>>>>
>>>>> to add this yet another confusing property.
>>>>
>>>> you not adding another property, you fix existent
>>>> (which was incorrectly used/implemented).
>>>>
>>>>>
>>>>> BTW - If a requirement will get from the field to add properties
>>>>> we
>>>>> can
>>>>> do them later why add something we think is not needed.
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>>
>>>> Michael Pasternak
>>>> RedHat, ENG-Virtualization R&D
>>>>
>>>
>>> I think we got a little bit off the topic here, so if you don't
>>> mind I would like to see if everyone agrees on this:
>>>
>>> We have at the api/networks collection these properties and their
>>> possible values:
>>>   status - OPERATIONAL, NON_OPERATIONAL
>>>   display - true, false
>>>
>>> We (as far as I understood) agreed that these fields causea problem
>>> in this context since they can be different for a given network,
>>> and current representation will return the network element
>>> multiple times with only difference in either one of these fields.
>>> Also I understood we agreed that this is bad behaviour (even a bug)
>>> and we don't want to support this anymore.
>>>
>>> This gives 2 choices IMHO:
>>>   1. Fix the behaviour but keep the fields with some default
>>>   values.
>>>   2. Fix the behaviour and remove these field as well, which isn't
>>>   really breaking an API since the behaviour was broken to begin
>>>   with.
>>>
>>
>> So a summary of the thread so far:
>>
>> Simon, Miki Ori and me voted +1 for option #2
>>
>> Michael wants to change the value of the status field to
>> attach/detach
>>
>> Anyone else wants to vote in on this?
> 
> I vote for fix #2.
> 
> I think not only is leaving these fields with some defaults a mistake, but also changing their possible values is breaking the API either way, so if we already breaking the API I think removing the fields entirely is cleaner, and in future if we have request to add fields then we can model them correctly.

+1 for #2 (but only for <display> and new 3.1 props),
-2 for removing <status> (based on told above)

> 
>>
>>
>>> Please comment what option seems valid (I though we were going to
>>> the direction of fix #2).
>>>
>>> Thanks,
>>> Mike
>>>
>>
>>
>>


-- 

Michael Pasternak
RedHat, ENG-Virtualization R&D