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

[Livnat Peer]

On 05/07/12 12:13, Michael Pasternak wrote:
> On 07/05/2012 12:06 PM, Livnat Peer wrote:
>> On 05/07/12 11:56, Michael Pasternak wrote:
>>> On 07/05/2012 11:40 AM, Livnat Peer wrote:
>>>> On 05/07/12 11:31, Michael Pasternak wrote:
>>>>> On 07/05/2012 10:51 AM, Livnat Peer wrote:
>>>>>>>>>> Actually the API has the same concept as you suggest for storage
>>>>>>>>>>>>>> domains.
>>>>>>>>>>>>>> At the top level you don't have a status field, but under data
>>>>>>>>>>>>>> center level, where it's valid then you get the status property.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Same should go for networks.
>>>>>>>>>>>>>> The status property should be added only where it's valid, in
>>>>>>>>>>>>>> this
>>>>>>>>>>>>>> case the cluster level sub-collection
>>>>>>>>>>>>
>>>>>>>>>>>> so sounds like we want to declare these properties deprecated to be
>>>>>>>>>>>> able
>>>>>>>>>>>> to remove them in a future version?
>>>>>>>>>>
>>>>>>>>>> I guess so,
>>>>>>>>>> The question is, are there other location where the status property
>>>>>>>>>> (or any other property) exists at an irrelevant level. Unless we
>>>>>>>>>> want to go into the effort of mapping them all now we probably need
>>>>>>>>>> to define a concept and anything not complying to that is a bug that
>>>>>>>>>> is allowed to be fixed without considering it as breaking the API.
>>>>>>>>>>
>>>>>>>>>> Thoughts?
>>>>>>>>>>
>>>>>>>> +1 
>>>>>>>> I agree that this is a bug and I DO suggest we  go into the effort of reviewing the other objects as well.
>>>>>>>> This is too major to just fix this one, and wait until we bump into another one...
>>>>>> Mike i see there a general consensus that this is a bug and the top
>>>>>> level entity should be a DC network.
>>>>>
>>>>> i disagree that <status> should be completely removed, instead as bug fix it
>>>>> should contain different members: ATTACHED|UNATTACHED (same concept we using in
>>>>> /api/storagedomains/xxx)
>>>>
>>>> first you agree we should remove the status as it is today as it does
>>>> not indicate anything to the user.
>>>
>>> http://lists.ovirt.org/pipermail/engine-devel/2012-July/002009.html
>>>
>>>>
>>>> second you suggest that we'll add attached unattached status, I don't
>>>> see value in it unless you specify the clusters it is attached to as a
>>>> sub - collection, I don't see us getting to this anytime soon.
>>>
>>> exactly on opposite, if network would have /clusters links sub-collection,
>>> <status>attached|unattached<status> will not be needed as it obvious by
>>> absence or existence of clusters links in sub-collection,
>>>
>>> the use-case is: when you have N networks in DC and want to find unused one
>>> to attach it to cluster.
>>>
>>
>> I don't see the logic in this use case, you don't attach a network to a
>> cluster because it is unused, you attach it because you want to use it,
>> having it attached to another cluster does not make much of a difference.
>>
>> I don't see the need for the attached/detached property.
>> I do think that adding a sub-collection of attached cluster can give
>> some value to the user but again not an immediate action.
> 
> 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 to add this yet another
confusing property.

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.


>>
>>
>>> (without this <status> you'll have to traverse over all networks against all
>>> clusters to find one unused)
>>>
>>>>
>>>> we can always add it later and it does not change the fact that the API
>>>
>>> the solution is very simple and does not require any resources:
>>>
>>> 1. to enum NetworkStatus add new (default) member UNATTACHED
>>> 2. clients will show UNATTACHED if NetworkStatus == UNATTACHED
>>>    or ATTACHED otherwise
>>>
>>>> changes.
>>>>
>>>>
>>>>>
>>>>>> Can you please open a bug / update the existing bug to reflect that.
>>>>>>
>>>>>> Thanks, Livnat
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>
>>
> 
>