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

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.

> (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 >>>> >>>> >>>> >>> >>> >> >> > >