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

[Oved Ourfalli]

----- Original Message -----
> From: "Mike Kolesnik" <mkolesni at redhat.com>
> To: "engine-devel" <engine-devel at ovirt.org>
> Cc: "Simon Grinberg" <simon at redhat.com>
> Sent: Thursday, July 5, 2012 1:43:26 PM
> Subject: Re: [Engine-devel] Fwd: Problem in REST API handling/displaying of	logical networks
> 
> ----- 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.
> 
I vote for fix #2 as well.
It makes more sense, as IMO there is no need to display cluster-specific fields on the top-level network collection.
Mixing cluster related fields in the logical network resource is wrong as it mixes two abstraction layers in one resource.
Also, looks like logical networks will become main entities in the future, which also strengthen this approach.

> > 
> > 
> > > Please comment what option seems valid (I though we were going to
> > > the direction of fix #2).
> > > 
> > > Thanks,
> > > Mike
> > > 
> > 
> > 
> > 
> _______________________________________________
> Engine-devel mailing list
> Engine-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
>