9:35 a.m.
--=_4ee2f20c-97ca-4df5-82e5-22b48c2bc7b8
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Hi All,
I would like to hear opinions about a behaviour that I think is problematic in
REST API handling of logical networks.
-- Intro --
Today in the REST API we are exposing two collections for "logical
network" related entities.
First is a top level collection which is out of any context at the address
http://engine/api/networks.
Second is a sub-collection in the context of a cluster:
http://engine/api/cluster/xxx/networks
The network itself is defined per DC level, so for each DC you would have
at least one logical network for management, which has some properties such
as STP, MTU, etc..
The top level collection is used to create/delete such network entities.
The sub-collection in the context of a Cluster is used to attach/detach a
network from the DC of that cluster.
The network in the context of a cluster has some additional information, let's
say for example 'status' of the network:
If a network is defined on all hosts in the cluster then it's status is
'Operational'.
If a network is not defined on some of the hosts in the cluster then it's
status is 'Not Operational'[1].
-- Problem --
The problem is that details which are only relevant in context of a
cluster, are still displayed in the root context as well (e.g: 'status').
This can, in certain cases, cause unexpected behaviour.
For example, let's consider this topology:
Data Center A
|
|\____ Network 'red'
|\____ Cluster A1
| \______ Network 'red' attached
\____ Cluster A2
\______ Network 'red' attached
If the 'status' is the same on all the clusters that the network is attached to
(A1, A2) then there will be one element in the top level collection, with the
network details and the 'status' field representing the state (which is same
for all networks in the cluster contexts of the cluster).
If, however, the status is not the same (ie. on A1 the network is
'Operational' and on A2 it is 'Non Operational') then the top-level
collection will show two elements for the network, where all network
details are the same and only the 'status' field is different.
This is problematic IMHO for several reasons:
1. Showing one network in certain states, and multiple copies of this
network in other states is not optimal, to say the least.
2. In the top-level collection there is no indicator of the cluster for which
the network is displayed, so there is no way to differentiate between the
two 'red' network elements (they will have same id, name, etc.).
3. There is a certain asymmetry between the remove action[2] and the
result in that you would expect: you either remove a network but in the
result you would see several elements removed.
-- Proposed Solutions --
Personally I can think of several solutions to this problem:
1. Declare the top-level collection as a collection of all networks that are
either attached to cluster or not, and if they are indeed attached then
show the details for each cluster, including a link to the cluster.
2. Declare the top-level collection as a collection of all networks that are
defined in data-centers, but they will not contain any cluster specific
data, and thus each entry is unique.
Solution #2 is breaking the API backwards-compatibility, since it includes
removing certain fields that have appeared today (namely 'status' and
'display') but IMO would give a better experience since the top-level
collection is actually used for managing networks, and not their attachment
to clusters which should be done in the context of each cluster.
I would like to hear what suggestions you have to solve this problem or if
you prefer either of the above solutions.
-- Footnotes --
[1] In 3.1 this is slightly different, but for the sake of simplicity I didn't
specify the new behaviour.
[2] Currently you can't update the network if it's attached to any cluster,
but perhaps in the future this would be possible.
Regards,
Mike
--=_4ee2f20c-97ca-4df5-82e5-22b48c2bc7b8
Content-Type: text/html; charset=utf-8
Content-Transfer-Encoding: quoted-printable
<html><head><style type=3D'text/css'>p { margin: 0;
}</style></head><body><=
div style=3D'font-family: Times New Roman; font-size: 12pt; color: #000000'=
<div
style=3D"color:#000;font-weight:normal;font-style:normal;text-decorat=
ion:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><style></s=
tyle><div style=3D"font-family: Times New Roman; font-size: 12pt; color: #0=
00000">Hi All,<br><br>I would like to hear opinions about a behaviour
that =
I think is problematic in<br>REST API handling of logical
networks.<br><br>=
-- Intro --<br>Today in the REST API we are exposing two collections for "l=
ogical<br>network" related entities.<br><br>First is a top level
collection=
which is out of any context at the address <br>http://engine/api/networks.=
<br>Second is a sub-collection in the context of a cluster: <br>http://engi=
ne/api/cluster/xxx/networks<br><br>The network itself is defined per DC lev=
el, so for each DC you would have <br>at least one logical network for mana=
gement, which has some properties such<br>as STP, MTU, etc..<br>The top lev=
el collection is used to create/delete such network entities.<br><br>The su=
b-collection in the context of a Cluster is used to attach/detach a <br>net=
work from the DC of that cluster.<br>The network in the context of a cluste=
r has some additional information, let's<br>say for example 'status' of
the=
network:<br> If a network is defined on all hosts in the
clus=
ter then it's status is <br>
'Operational'.<br> &nb=
sp; If a network is not defined on some of the hosts in the cluster then it=
's <br> status is 'Not
Operational'[1].<br><br><br>-- Pro=
blem --<br>The problem is that details which are only relevant in context o=
f a<br>cluster, are still displayed in the root context as well (e.g: 'stat=
us').<br>
This can, in certain cases, cause unexpected behaviour.<br><br>For example,=
let's consider this topology:<br> Data Center
A<br>  =
;
|<br> |=
\____ Network 'red'<br>
|\____ Cluster A1<b=
r>
| &nb=
sp; \______ Network
'red' attached<br>&=
nbsp; \____ Cluster
A2<br>  =
; &n=
bsp; \______ Network 'red'
attached<br><br>If=
the 'status' is the same on all the clusters that the network is attached =
to<br>(A1, A2) then there will be one element in the top level collection, =
with the<br>network details and the 'status' field representing the state
(=
which is same<br>for all networks in the cluster contexts of the cluster).<=
br>If, however, the status is not the same (ie. on A1 the network is<br>'Op=
erational' and on A2 it is 'Non Operational') then the top-level
<br>collec=
tion will show two elements for the network, where all network<br>details a=
re the same and only the 'status' field is different.<br><br>This is
proble=
matic IMHO for several reasons:<br> 1. Showing one network in certain=
states, and multiple copies of
this<br> netw=
ork in other states is not optimal, to say the least.<br> 2. In the t=
op-level collection there is no indicator of the cluster for which<br> =
; the network is displayed, so there is no way to
d=
ifferentiate between the<br> two
'red' networ=
k elements (they will have same id, name, etc.).<br> 3. There is a ce=
rtain asymmetry between the remove action[2] and the
<br> =
result in that you would expect: you either remove a network b=
ut in the<br> result you would see
several el=
ements removed.<br><br><br>-- Proposed Solutions --<br>Personally
I can thi=
nk of several solutions to this problem:<br> 1. Declare the top-level=
collection as a collection of all networks that
are<br> &=
nbsp; either attached to cluster or not, and if they are indeed attac=
hed then<br> show the details for
each cluste=
r, including a link to the cluster.<br> 2. Declare the top-level coll=
ection as a collection of all networks that
are<br> =
defined in data-centers, but they will not contain any cluster speci=
fic<br> data, and thus each entry
is unique.<=
br><br>Solution #2 is breaking the API backwards-compatibility, since it in=
cludes<br>removing certain fields that have appeared today (namely
'status'=
and<br>'display') but IMO would give a better experience since the
top-lev=
el<br>collection is actually used for managing networks, and not their atta=
chment<br>to clusters which should be done in the context of each cluster.<=
br><br>I would like to hear what suggestions you have to solve this problem=
or if<br>you prefer either of the above solutions.<br><br><br>--
Footnotes=
--<br>[1] In 3.1 this is slightly different, but for the sake of simplicit=
y I didn't<br> specify the new
behaviour.<br>[2] Cu=
rrently you can't update the network if it's attached to any
cluster,<br>&n=
bsp; but perhaps in the future this would be
possible.<br=
<br><div><span></span>Regards,<br>Mike<span></span><br></div><br></div></d=
iv><br></div></body></html>
--=_4ee2f20c-97ca-4df5-82e5-22b48c2bc7b8--
1:51 a.m.
New subject: [Engine-devel] Fwd: Problem in REST API handling/displaying of logical networks
Comments inline
----- Original Message -----
From: "Mike Kolesnik" <mkolesni(a)redhat.com>
To: "engine-devel" <engine-devel(a)ovirt.org>
Sent: Monday, July 2, 2012 5:35:52 PM
Subject: [Engine-devel] Fwd: Problem in REST API handling/displaying of logical networks
Hi All,
I would like to hear opinions about a behaviour that I think is
problematic in
REST API handling of logical networks.
-- Intro --
Today in the REST API we are exposing two collections for "logical
network" related entities.
First is a top level collection which is out of any context at the
address
http://engine/api/networks.
Second is a sub-collection in the context of a cluster:
http://engine/api/cluster/xxx/networks
The network itself is defined per DC level, so for each DC you would
have
at least one logical network for management, which has some
properties such
as STP, MTU, etc..
The top level collection is used to create/delete such network
entities.
The sub-collection in the context of a Cluster is used to
attach/detach a
network from the DC of that cluster.
Edit as well. you can update cluster network and change the "Display" property.
The network in the context of a cluster has some additional
information, let's
say for example 'status' of the network:
If a network is defined on all hosts in the cluster then it's status
is
'Operational'.
If a network is not defined on some of the hosts in the cluster then
it's
status is 'Not Operational'[1].
-- Problem --
The problem is that details which are only relevant in context of a
cluster, are still displayed in the root context as well (e.g:
'status').
This can, in certain cases, cause unexpected behaviour.
+1 no need to display cluster properties in datacenter context.
(https://bugzilla.redhat.com/show_bug.cgi?id=815268)
For example, let's consider this topology:
Data Center A
|
|\____ Network 'red'
|\____ Cluster A1
| \______ Network 'red' attached
\____ Cluster A2
\______ Network 'red' attached
If the 'status' is the same on all the clusters that the network is
attached to
(A1, A2) then there will be one element in the top level collection,
with the
network details and the 'status' field representing the state (which
is same
for all networks in the cluster contexts of the cluster).
If, however, the status is not the same (ie. on A1 the network is
'Operational' and on A2 it is 'Non Operational') then the top-level
collection will show two elements for the network, where all network
details are the same and only the 'status' field is different.
This is problematic IMHO for several reasons:
1. Showing one network in certain states, and multiple copies of this
network in other states is not optimal, to say the least.
2. In the top-level collection there is no indicator of the cluster
for which
the network is displayed, so there is no way to differentiate between
the
two 'red' network elements (they will have same id, name, etc.).
3. There is a certain asymmetry between the remove action[2] and the
result in that you would expect: you either remove a network but in
the
result you would see several elements removed.
-- Proposed Solutions --
Personally I can think of several solutions to this problem:
1. Declare the top-level collection as a collection of all networks
that are
either attached to cluster or not, and if they are indeed attached
then
show the details for each cluster, including a link to the cluster.
2. Declare the top-level collection as a collection of all networks
that are
defined in data-centers, but they will not contain any cluster
specific
data, and thus each entry is unique.
Why do we keep holding this Top level collection instead of having each network related
under his datacenter (/api/datacenter/id/networks)???
https://bugzilla.redhat.com/show_bug.cgi?id=741111
One solution which is must is to move the top level network (/api/networks) under each
Datacenter!
1. having top level networks path isn't symmetric to our entire REST API
implementation!
all other network collections (cluster, hosts, VMs, Templates) placed under their
related component.
2. as a REST API user, having sub collection networks under each datacenter is better to
handle than parsing a top level collection
and comparing DC id in order to get all DC related networks.
3. as for breaking api/ backward compatibility, we can still hold the top level network
collection ad deprecated and start exposing the sub collections
under each datacenter.
Solution #2 is breaking the API backwards-compatibility, since it
includes
removing certain fields that have appeared today (namely 'status' and
'display') but IMO would give a better experience since the top-level
collection is actually used for managing networks, and not their
attachment
to clusters which should be done in the context of each cluster.
I would like to hear what suggestions you have to solve this problem
or if
you prefer either of the above solutions.
-- Footnotes --
[1] In 3.1 this is slightly different, but for the sake of simplicity
I didn't
specify the new behaviour.
[2] Currently you can't update the network if it's attached to any
cluster,
but perhaps in the future this would be possible.
Regards,
Mike
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
2 a.m.
On 07/03/2012 09:51 AM, Avi Tal wrote:
...
Why do we keep holding this Top level collection instead of having each network related
under his datacenter (/api/datacenter/id/networks)???
why do we keep VMs as top level? they only exist in data center...
why do we keep Clusters as top level? they only exist in data center...
why do we keep Templates as top level? they only exist in data center...
all entities are available today as top level as well, so networks are
there for consistency
https://bugzilla.redhat.com/show_bug.cgi?id=741111
One solution which is must is to move the top level network (/api/networks) under each
Datacenter!
1. having top level networks path isn't symmetric to our entire REST API
implementation!
all other network collections (cluster, hosts, VMs, Templates) placed under their
related component.
2. as a REST API user, having sub collection networks under each datacenter is better to
handle than parsing a top level collection
and comparing DC id in order to get all DC related networks.
3. as for breaking api/ backward compatibility, we can still hold the top level network
collection ad deprecated and start exposing the sub collections
under each datacenter.
2:06 a.m.
On 07/03/2012 10:00 AM, Itamar Heim wrote:
On 07/03/2012 09:51 AM, Avi Tal wrote:
...
>
> Why do we keep holding this Top level collection instead of having
> each network related under his datacenter
> (/api/datacenter/id/networks)???
why do we keep VMs as top level? they only exist in data center...
why do we keep Clusters as top level? they only exist in data center...
why do we keep Templates as top level? they only exist in data center...
What's the answer to all the above, btw?
Y.
all entities are available today as top level as well, so networks are
there for consistency
>
> https://bugzilla.redhat.com/show_bug.cgi?id=741111
>
> One solution which is must is to move the top level network
> (/api/networks) under each Datacenter!
>
> 1. having top level networks path isn't symmetric to our entire REST
> API implementation!
> all other network collections (cluster, hosts, VMs, Templates)
> placed under their related component.
> 2. as a REST API user, having sub collection networks under each
> datacenter is better to handle than parsing a top level collection
> and comparing DC id in order to get all DC related networks.
> 3. as for breaking api/ backward compatibility, we can still hold the
> top level network collection ad deprecated and start exposing the sub
> collections
> under each datacenter.
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
2:30 a.m.
On 02/07/12 17:35, Mike Kolesnik wrote:
Hi All,
I would like to hear opinions about a behaviour that I think is
problematic in
REST API handling of logical networks.
-- Intro --
Today in the REST API we are exposing two collections for "logical
network" related entities.
First is a top level collection which is out of any context at the address
http://engine/api/networks.
Second is a sub-collection in the context of a cluster:
http://engine/api/cluster/xxx/networks
The network itself is defined per DC level, so for each DC you would have
at least one logical network for management, which has some properties such
as STP, MTU, etc..
The top level collection is used to create/delete such network entities.
The sub-collection in the context of a Cluster is used to attach/detach a
network from the DC of that cluster.
The network in the context of a cluster has some additional information,
let's
say for example 'status' of the network:
If a network is defined on all hosts in the cluster then it's status is
'Operational'.
If a network is not defined on some of the hosts in the cluster then
it's
status is 'Not Operational'[1].
-- Problem --
The problem is that details which are only relevant in context of a
cluster, are still displayed in the root context as well (e.g: 'status').
This can, in certain cases, cause unexpected behaviour.
For example, let's consider this topology:
Data Center A
|
|\____ Network 'red'
|\____ Cluster A1
| \______ Network 'red' attached
\____ Cluster A2
\______ Network 'red' attached
If the 'status' is the same on all the clusters that the network is
attached to
(A1, A2) then there will be one element in the top level collection,
with the
network details and the 'status' field representing the state (which is same
for all networks in the cluster contexts of the cluster).
If, however, the status is not the same (ie. on A1 the network is
'Operational' and on A2 it is 'Non Operational') then the top-level
collection will show two elements for the network, where all network
details are the same and only the 'status' field is different.
That sounds like a bug to me.
I think that top collection should include only DC level properties and
not cluster level properties, status should not be there (same as
display required etc.)
This is problematic IMHO for several reasons:
1. Showing one network in certain states, and multiple copies of this
network in other states is not optimal, to say the least.
2. In the top-level collection there is no indicator of the cluster
for which
the network is displayed, so there is no way to differentiate
between the
two 'red' network elements (they will have same id, name, etc.).
3. There is a certain asymmetry between the remove action[2] and the
result in that you would expect: you either remove a network but
in the
result you would see several elements removed.
-- Proposed Solutions --
Personally I can think of several solutions to this problem:
1. Declare the top-level collection as a collection of all networks
that are
either attached to cluster or not, and if they are indeed attached
then
show the details for each cluster, including a link to the cluster.
2. Declare the top-level collection as a collection of all networks
that are
defined in data-centers, but they will not contain any cluster
specific
data, and thus each entry is unique.
Solution #2 is breaking the API backwards-compatibility, since it includes
removing certain fields that have appeared today (namely 'status' and
'display') but IMO would give a better experience since the top-level
collection is actually used for managing networks, and not their attachment
to clusters which should be done in the context of each cluster.
I really don't think top collections should include cluster networks it
is not user-friendly to say the least.
I vote for the second option, I don't think that having a bug in
previous versions should drive this decision.
I would like to hear what suggestions you have to solve this problem
or if
you prefer either of the above solutions.
-- Footnotes --
[1] In 3.1 this is slightly different, but for the sake of simplicity I
didn't
specify the new behaviour.
[2] Currently you can't update the network if it's attached to any cluster,
but perhaps in the future this would be possible.
Regards,
Mike
2:54 a.m.
On 07/02/2012 05:35 PM, Mike Kolesnik wrote:
Hi All,
I would like to hear opinions about a behaviour that I think is problematic in
REST API handling of logical networks.
-- Intro --
Today in the REST API we are exposing two collections for "logical
network" related entities.
First is a top level collection which is out of any context at the address
http://engine/api/networks.
Second is a sub-collection in the context of a cluster:
http://engine/api/cluster/xxx/networks
The network itself is defined per DC level, so for each DC you would have
at least one logical network for management, which has some properties such
as STP, MTU, etc..
The top level collection is used to create/delete such network entities.
The sub-collection in the context of a Cluster is used to attach/detach a
network from the DC of that cluster.
The network in the context of a cluster has some additional information, let's
say for example 'status' of the network:
If a network is defined on all hosts in the cluster then it's status is
'Operational'.
If a network is not defined on some of the hosts in the cluster then it's
status is 'Not Operational'[1].
-- Problem --
The problem is that details which are only relevant in context of a
cluster, are still displayed in the root context as well (e.g: 'status').
this is 2.2 bug, behaviour has to be the same as in /api/storagedomains
and /api/datacenters/xxx/storagedomains - 'status' should be addressed indeed.
(only use-case for status in /api/networks is when network yet not attached
to any cluster)
This can, in certain cases, cause unexpected behaviour.
For example, let's consider this topology:
Data Center A
|
|\____ Network 'red'
|\____ Cluster A1
| \______ Network 'red' attached
\____ Cluster A2
\______ Network 'red' attached
If the 'status' is the same on all the clusters that the network is attached to
(A1, A2) then there will be one element in the top level collection, with the
network details and the 'status' field representing the state (which is same
for all networks in the cluster contexts of the cluster).
If, however, the status is not the same (ie. on A1 the network is
'Operational' and on A2 it is 'Non Operational') then the top-level
collection will show two elements for the network, where all network
details are the same and only the 'status' field is different.
This is problematic IMHO for several reasons:
1. Showing one network in certain states, and multiple copies of this
network in other states is not optimal, to say the least.
2. In the top-level collection there is no indicator of the cluster for which
the network is displayed, so there is no way to differentiate between the
two 'red' network elements (they will have same id, name, etc.).
3. There is a certain asymmetry between the remove action[2] and the
result in that you would expect: you either remove a network but in the
result you would see several elements removed.
this is not correct, remove from /api/networks is not considered as *detach* from
clusters as well, but only as remove from /api/networks which is deletes network
from the DC, so it can't be 1:N,
if network still attached to cluster/s, backend should block this operation by CDA,
this is not api prerogative anyway.
-- Proposed Solutions --
Personally I can think of several solutions to this problem:
1. Declare the top-level collection as a collection of all networks that are
either attached to cluster or not, and if they are indeed attached then
show the details for each cluster, including a link to the cluster.
2. Declare the top-level collection as a collection of all networks that are
defined in data-centers, but they will not contain any cluster specific
data, and thus each entry is unique.
Solution #2 is breaking the API backwards-compatibility,
since it includes
removing certain fields that have appeared today (namely 'status' and
'display') but IMO would give a better experience since the top-level
collection is actually used for managing networks, and not their attachment
to clusters which should be done in the context of each cluster.
i vote for #2, that's the right way to go forward (but only if leaving properties
in /api/networks creates logical collisions as in 'status' case)
as for now it's only: 'status', 'display', right?
I would like to hear what suggestions you have to solve this problem or if
you prefer either of the above solutions.
-- Footnotes --
[1] In 3.1 this is slightly different, but for the sake of simplicity I didn't
specify the new behaviour.
[2] Currently you can't update the network if it's attached to any cluster,
but perhaps in the future this would be possible.
Regards,
Mike
--
Michael Pasternak
RedHat, ENG-Virtualization R&D
3:20 a.m.
On 07/03/2012 10:06 AM, Yaniv Kaul wrote:
On 07/03/2012 10:00 AM, Itamar Heim wrote:
> On 07/03/2012 09:51 AM, Avi Tal wrote:
> ...
>>
>> Why do we keep holding this Top level collection instead of having each network
related under his datacenter (/api/datacenter/id/networks)???
>
> why do we keep VMs as top level? they only exist in data center...
> why do we keep Clusters as top level? they only exist in data center...
> why do we keep Templates as top level? they only exist in data center...
What's the answer to all the above, btw?
Y.
1. they are central entities in the system and should be easily accessed - simplicity.
2. ease on system initiation, i.e creating same sub-resource for different
resources is much more easy when you don't have to switch resource
collection every time.
3. hypermedia is the engine of the application state (one of the core REST principals).
>
> all entities are available today as top level as well, so networks are there for
consistency
>
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=741111
>>
>> One solution which is must is to move the top level network (/api/networks) under
each Datacenter!
not *move*, but to be added as well, we have RFE on this.
>>
>> 1. having top level networks path isn't symmetric to our entire REST API
implementation!
>> all other network collections (cluster, hosts, VMs, Templates) placed under
their related component.
>> 2. as a REST API user, having sub collection networks under each datacenter is
better to handle than parsing a top level collection
>> and comparing DC id in order to get all DC related networks.
>> 3. as for breaking api/ backward compatibility, we can still hold the top level
network collection ad deprecated and start exposing the sub collections
>> under each datacenter.
> _______________________________________________
> Engine-devel mailing list
> Engine-devel(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
--
Michael Pasternak
RedHat, ENG-Virtualization R&D
5:21 a.m.
----- Original Message -----
From: "Itamar Heim" <iheim(a)redhat.com>
To: "Avi Tal" <atal(a)redhat.com>
Cc: "Mike Kolesnik" <mkolesni(a)redhat.com>, "engine-devel"
<engine-devel(a)ovirt.org>
Sent: Tuesday, July 3, 2012 10:00:38 AM
Subject: Re: [Engine-devel] Fwd: Problem in REST API handling/displaying of logical
networks
On 07/03/2012 09:51 AM, Avi Tal wrote:
...
>
> Why do we keep holding this Top level collection instead of having
> each network related under his datacenter
> (/api/datacenter/id/networks)???
why do we keep VMs as top level? they only exist in data center...
why do we keep Clusters as top level? they only exist in data
center...
why do we keep Templates as top level? they only exist in data
center...
all entities are available today as top level as well, so networks
are
there for consistency
So why don't we expose Network as TAB in rhevm UI? (we chose to manage it under DC)
>
> https://bugzilla.redhat.com/show_bug.cgi?id=741111
>
> One solution which is must is to move the top level network
> (/api/networks) under each Datacenter!
>
> 1. having top level networks path isn't symmetric to our entire
> REST API implementation!
> all other network collections (cluster, hosts, VMs, Templates)
> placed under their related component.
> 2. as a REST API user, having sub collection networks under each
> datacenter is better to handle than parsing a top level collection
> and comparing DC id in order to get all DC related networks.
> 3. as for breaking api/ backward compatibility, we can still hold
> the top level network collection ad deprecated and start exposing
> the sub collections
> under each datacenter.
5:36 a.m.
On 07/03/2012 01:21 PM, Avi Tal wrote:
----- Original Message -----
> From: "Itamar Heim" <iheim(a)redhat.com>
> To: "Avi Tal" <atal(a)redhat.com>
> Cc: "Mike Kolesnik" <mkolesni(a)redhat.com>, "engine-devel"
<engine-devel(a)ovirt.org>
> Sent: Tuesday, July 3, 2012 10:00:38 AM
> Subject: Re: [Engine-devel] Fwd: Problem in REST API handling/displaying of logical
networks
>
> On 07/03/2012 09:51 AM, Avi Tal wrote:
> ...
>>
>> Why do we keep holding this Top level collection instead of having
>> each network related under his datacenter
>> (/api/datacenter/id/networks)???
>
> why do we keep VMs as top level? they only exist in data center...
> why do we keep Clusters as top level? they only exist in data
> center...
> why do we keep Templates as top level? they only exist in data
> center...
>
> all entities are available today as top level as well, so networks
> are
> there for consistency
So why don't we expose Network as TAB in rhevm UI? (we chose to manage it under DC)
coming soon to an ovirt near you...
>
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=741111
>>
>> One solution which is must is to move the top level network
>> (/api/networks) under each Datacenter!
>>
>> 1. having top level networks path isn't symmetric to our entire
>> REST API implementation!
>> all other network collections (cluster, hosts, VMs, Templates)
>> placed under their related component.
>> 2. as a REST API user, having sub collection networks under each
>> datacenter is better to handle than parsing a top level collection
>> and comparing DC id in order to get all DC related networks.
>> 3. as for breaking api/ backward compatibility, we can still hold
>> the top level network collection ad deprecated and start exposing
>> the sub collections
>> under each datacenter.
>
1:28 p.m.
On 07/03/2012 10:30 AM, Livnat Peer wrote:
On 02/07/12 17:35, Mike Kolesnik wrote:
> Hi All,
>
> I would like to hear opinions about a behaviour that I think is
> problematic in
> REST API handling of logical networks.
>
> -- Intro --
> Today in the REST API we are exposing two collections for "logical
> network" related entities.
>
> First is a top level collection which is out of any context at the address
> http://engine/api/networks.
> Second is a sub-collection in the context of a cluster:
> http://engine/api/cluster/xxx/networks
>
> The network itself is defined per DC level, so for each DC you would have
> at least one logical network for management, which has some properties such
> as STP, MTU, etc..
> The top level collection is used to create/delete such network entities.
>
> The sub-collection in the context of a Cluster is used to attach/detach a
> network from the DC of that cluster.
> The network in the context of a cluster has some additional information,
> let's
> say for example 'status' of the network:
> If a network is defined on all hosts in the cluster then it's status is
> 'Operational'.
> If a network is not defined on some of the hosts in the cluster then
> it's
> status is 'Not Operational'[1].
>
>
> -- Problem --
> The problem is that details which are only relevant in context of a
> cluster, are still displayed in the root context as well (e.g: 'status').
> This can, in certain cases, cause unexpected behaviour.
>
> For example, let's consider this topology:
> Data Center A
> |
> |\____ Network 'red'
> |\____ Cluster A1
> | \______ Network 'red' attached
> \____ Cluster A2
> \______ Network 'red' attached
>
> If the 'status' is the same on all the clusters that the network is
> attached to
> (A1, A2) then there will be one element in the top level collection,
> with the
> network details and the 'status' field representing the state (which is same
> for all networks in the cluster contexts of the cluster).
> If, however, the status is not the same (ie. on A1 the network is
> 'Operational' and on A2 it is 'Non Operational') then the top-level
> collection will show two elements for the network, where all network
> details are the same and only the 'status' field is different.
>
That sounds like a bug to me.
I think that top collection should include only DC level properties and
not cluster level properties, status should not be there (same as
display required etc.)
> This is problematic IMHO for several reasons:
> 1. Showing one network in certain states, and multiple copies of this
> network in other states is not optimal, to say the least.
> 2. In the top-level collection there is no indicator of the cluster
> for which
> the network is displayed, so there is no way to differentiate
> between the
> two 'red' network elements (they will have same id, name, etc.).
> 3. There is a certain asymmetry between the remove action[2] and the
> result in that you would expect: you either remove a network but
> in the
> result you would see several elements removed.
>
>
> -- Proposed Solutions --
> Personally I can think of several solutions to this problem:
> 1. Declare the top-level collection as a collection of all networks
> that are
> either attached to cluster or not, and if they are indeed attached
> then
> show the details for each cluster, including a link to the cluster.
> 2. Declare the top-level collection as a collection of all networks
> that are
> defined in data-centers, but they will not contain any cluster
> specific
> data, and thus each entry is unique.
>
> Solution #2 is breaking the API backwards-compatibility, since it includes
> removing certain fields that have appeared today (namely 'status' and
> 'display') but IMO would give a better experience since the top-level
> collection is actually used for managing networks, and not their attachment
> to clusters which should be done in the context of each cluster.
>
I really don't think top collections should include cluster networks it
is not user-friendly to say the least.
how is that different from top collections including VMs and templates?
(or logical networks becoming main tab in the UI going forward)
I vote for the second option, I don't think that having a bug in
previous versions should drive this decision.
> I would like to hear what suggestions you have to solve this problem or if
> you prefer either of the above solutions.
>
>
> -- Footnotes --
> [1] In 3.1 this is slightly different, but for the sake of simplicity I
> didn't
> specify the new behaviour.
> [2] Currently you can't update the network if it's attached to any cluster,
> but perhaps in the future this would be possible.
>
> Regards,
> Mike
>
>
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
2:06 p.m.
On 03/07/12 21:28, Itamar Heim wrote:
On 07/03/2012 10:30 AM, Livnat Peer wrote:
> On 02/07/12 17:35, Mike Kolesnik wrote:
>> Hi All,
>>
>> I would like to hear opinions about a behaviour that I think is
>> problematic in
>> REST API handling of logical networks.
>>
>> -- Intro --
>> Today in the REST API we are exposing two collections for "logical
>> network" related entities.
>>
>> First is a top level collection which is out of any context at the
>> address
>> http://engine/api/networks.
>> Second is a sub-collection in the context of a cluster:
>> http://engine/api/cluster/xxx/networks
>>
>> The network itself is defined per DC level, so for each DC you would
>> have
>> at least one logical network for management, which has some
>> properties such
>> as STP, MTU, etc..
>> The top level collection is used to create/delete such network entities.
>>
>> The sub-collection in the context of a Cluster is used to
>> attach/detach a
>> network from the DC of that cluster.
>> The network in the context of a cluster has some additional information,
>> let's
>> say for example 'status' of the network:
>> If a network is defined on all hosts in the cluster then it's
>> status is
>> 'Operational'.
>> If a network is not defined on some of the hosts in the cluster
>> then
>> it's
>> status is 'Not Operational'[1].
>>
>>
>> -- Problem --
>> The problem is that details which are only relevant in context of a
>> cluster, are still displayed in the root context as well (e.g:
>> 'status').
>> This can, in certain cases, cause unexpected behaviour.
>>
>> For example, let's consider this topology:
>> Data Center A
>> |
>> |\____ Network 'red'
>> |\____ Cluster A1
>> | \______ Network 'red' attached
>> \____ Cluster A2
>> \______ Network 'red' attached
>>
>> If the 'status' is the same on all the clusters that the network is
>> attached to
>> (A1, A2) then there will be one element in the top level collection,
>> with the
>> network details and the 'status' field representing the state (which
>> is same
>> for all networks in the cluster contexts of the cluster).
>> If, however, the status is not the same (ie. on A1 the network is
>> 'Operational' and on A2 it is 'Non Operational') then the
top-level
>> collection will show two elements for the network, where all network
>> details are the same and only the 'status' field is different.
>>
>
> That sounds like a bug to me.
> I think that top collection should include only DC level properties and
> not cluster level properties, status should not be there (same as
> display required etc.)
>
>
>> This is problematic IMHO for several reasons:
>> 1. Showing one network in certain states, and multiple copies of this
>> network in other states is not optimal, to say the least.
>> 2. In the top-level collection there is no indicator of the cluster
>> for which
>> the network is displayed, so there is no way to differentiate
>> between the
>> two 'red' network elements (they will have same id, name, etc.).
>> 3. There is a certain asymmetry between the remove action[2] and the
>> result in that you would expect: you either remove a network but
>> in the
>> result you would see several elements removed.
>>
>>
>> -- Proposed Solutions --
>> Personally I can think of several solutions to this problem:
>> 1. Declare the top-level collection as a collection of all networks
>> that are
>> either attached to cluster or not, and if they are indeed
>> attached
>> then
>> show the details for each cluster, including a link to the
>> cluster.
>> 2. Declare the top-level collection as a collection of all networks
>> that are
>> defined in data-centers, but they will not contain any cluster
>> specific
>> data, and thus each entry is unique.
>>
>> Solution #2 is breaking the API backwards-compatibility, since it
>> includes
>> removing certain fields that have appeared today (namely 'status' and
>> 'display') but IMO would give a better experience since the top-level
>> collection is actually used for managing networks, and not their
>> attachment
>> to clusters which should be done in the context of each cluster.
>>
> I really don't think top collections should include cluster networks it
> is not user-friendly to say the least.
how is that different from top collections including VMs and templates?
(or logical networks becoming main tab in the UI going forward)
I think you missed the point of cluster network entity vs. DC network
entity.
we should have in the top collection a DC network, I think we should not
display the network per cluster in top network collection (that can be
viewed under the cluster context).
>
> I vote for the second option, I don't think that having a bug in
> previous versions should drive this decision.
>
>
>> I would like to hear what suggestions you have to solve this problem
>> or if
>> you prefer either of the above solutions.
>>
>>
>> -- Footnotes --
>> [1] In 3.1 this is slightly different, but for the sake of simplicity I
>> didn't
>> specify the new behaviour.
>> [2] Currently you can't update the network if it's attached to any
>> cluster,
>> but perhaps in the future this would be possible.
>>
>> Regards,
>> Mike
>>
>>
>
>
> _______________________________________________
> Engine-devel mailing list
> Engine-devel(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
>
11:27 a.m.
----- Original Message -----
From: "Livnat Peer" <lpeer(a)redhat.com>
To: "Itamar Heim" <iheim(a)redhat.com>
Cc: "engine-devel" <engine-devel(a)ovirt.org>
Sent: Tuesday, July 3, 2012 10:06:37 PM
Subject: Re: [Engine-devel] Fwd: Problem in REST API handling/displaying of logical
networks
On 03/07/12 21:28, Itamar Heim wrote:
> On 07/03/2012 10:30 AM, Livnat Peer wrote:
>> On 02/07/12 17:35, Mike Kolesnik wrote:
>>> Hi All,
>>>
>>> I would like to hear opinions about a behaviour that I think is
>>> problematic in
>>> REST API handling of logical networks.
>>>
>>> -- Intro --
>>> Today in the REST API we are exposing two collections for
>>> "logical
>>> network" related entities.
>>>
>>> First is a top level collection which is out of any context at
>>> the
>>> address
>>> http://engine/api/networks.
>>> Second is a sub-collection in the context of a cluster:
>>> http://engine/api/cluster/xxx/networks
>>>
>>> The network itself is defined per DC level, so for each DC you
>>> would
>>> have
>>> at least one logical network for management, which has some
>>> properties such
>>> as STP, MTU, etc..
>>> The top level collection is used to create/delete such network
>>> entities.
>>>
>>> The sub-collection in the context of a Cluster is used to
>>> attach/detach a
>>> network from the DC of that cluster.
>>> The network in the context of a cluster has some additional
>>> information,
>>> let's
>>> say for example 'status' of the network:
>>> If a network is defined on all hosts in the cluster then
>>> it's
>>> status is
>>> 'Operational'.
>>> If a network is not defined on some of the hosts in the
>>> cluster
>>> then
>>> it's
>>> status is 'Not Operational'[1].
>>>
>>>
>>> -- Problem --
>>> The problem is that details which are only relevant in context of
>>> a
>>> cluster, are still displayed in the root context as well (e.g:
>>> 'status').
>>> This can, in certain cases, cause unexpected behaviour.
>>>
>>> For example, let's consider this topology:
>>> Data Center A
>>> |
>>> |\____ Network 'red'
>>> |\____ Cluster A1
>>> | \______ Network 'red' attached
>>> \____ Cluster A2
>>> \______ Network 'red' attached
>>>
>>> If the 'status' is the same on all the clusters that the network
>>> is
>>> attached to
>>> (A1, A2) then there will be one element in the top level
>>> collection,
>>> with the
>>> network details and the 'status' field representing the state
>>> (which
>>> is same
>>> for all networks in the cluster contexts of the cluster).
>>> If, however, the status is not the same (ie. on A1 the network is
>>> 'Operational' and on A2 it is 'Non Operational') then the
>>> top-level
>>> collection will show two elements for the network, where all
>>> network
>>> details are the same and only the 'status' field is different.
>>>
>>
>> That sounds like a bug to me.
>> I think that top collection should include only DC level
>> properties and
>> not cluster level properties, status should not be there (same as
>> display required etc.)
>>
>>
>>> This is problematic IMHO for several reasons:
>>> 1. Showing one network in certain states, and multiple copies
>>> of this
>>> network in other states is not optimal, to say the least.
>>> 2. In the top-level collection there is no indicator of the
>>> cluster
>>> for which
>>> the network is displayed, so there is no way to
>>> differentiate
>>> between the
>>> two 'red' network elements (they will have same id, name,
>>> etc.).
>>> 3. There is a certain asymmetry between the remove action[2]
>>> and the
>>> result in that you would expect: you either remove a
>>> network but
>>> in the
>>> result you would see several elements removed.
>>>
>>>
>>> -- Proposed Solutions --
>>> Personally I can think of several solutions to this problem:
>>> 1. Declare the top-level collection as a collection of all
>>> networks
>>> that are
>>> either attached to cluster or not, and if they are indeed
>>> attached
>>> then
>>> show the details for each cluster, including a link to the
>>> cluster.
>>> 2. Declare the top-level collection as a collection of all
>>> networks
>>> that are
>>> defined in data-centers, but they will not contain any
>>> cluster
>>> specific
>>> data, and thus each entry is unique.
>>>
>>> Solution #2 is breaking the API backwards-compatibility, since it
>>> includes
>>> removing certain fields that have appeared today (namely 'status'
>>> and
>>> 'display') but IMO would give a better experience since the
>>> top-level
>>> collection is actually used for managing networks, and not their
>>> attachment
>>> to clusters which should be done in the context of each cluster.
>>>
>> I really don't think top collections should include cluster
>> networks it
>> is not user-friendly to say the least.
>
> how is that different from top collections including VMs and
> templates?
> (or logical networks becoming main tab in the UI going forward)
>
I think you missed the point of cluster network entity vs. DC network
entity.
we should have in the top collection a DC network, I think we should
not
display the network per cluster in top network collection (that can
be
viewed under the cluster context).
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
>>
>> I vote for the second option, I don't think that having a bug in
>> previous versions should drive this decision.
>>
>>
>>> I would like to hear what suggestions you have to solve this
>>> problem
>>> or if
>>> you prefer either of the above solutions.
>>>
>>>
>>> -- Footnotes --
>>> [1] In 3.1 this is slightly different, but for the sake of
>>> simplicity I
>>> didn't
>>> specify the new behaviour.
>>> [2] Currently you can't update the network if it's attached to
>>> any
>>> cluster,
>>> but perhaps in the future this would be possible.
>>>
>>> Regards,
>>> Mike
>>>
>>>
>>
>>
>> _______________________________________________
>> Engine-devel mailing list
>> Engine-devel(a)ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/engine-devel
>>
>
>
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
11:38 a.m.
On 07/04/2012 07:27 PM, Simon Grinberg wrote:
----- Original Message -----
> From: "Livnat Peer" <lpeer(a)redhat.com>
> To: "Itamar Heim" <iheim(a)redhat.com>
> Cc: "engine-devel" <engine-devel(a)ovirt.org>
> Sent: Tuesday, July 3, 2012 10:06:37 PM
> Subject: Re: [Engine-devel] Fwd: Problem in REST API handling/displaying of logical
networks
>
> On 03/07/12 21:28, Itamar Heim wrote:
>> On 07/03/2012 10:30 AM, Livnat Peer wrote:
>>> On 02/07/12 17:35, Mike Kolesnik wrote:
>>>> Hi All,
>>>>
>>>> I would like to hear opinions about a behaviour that I think is
>>>> problematic in
>>>> REST API handling of logical networks.
>>>>
>>>> -- Intro --
>>>> Today in the REST API we are exposing two collections for
>>>> "logical
>>>> network" related entities.
>>>>
>>>> First is a top level collection which is out of any context at
>>>> the
>>>> address
>>>> http://engine/api/networks.
>>>> Second is a sub-collection in the context of a cluster:
>>>> http://engine/api/cluster/xxx/networks
>>>>
>>>> The network itself is defined per DC level, so for each DC you
>>>> would
>>>> have
>>>> at least one logical network for management, which has some
>>>> properties such
>>>> as STP, MTU, etc..
>>>> The top level collection is used to create/delete such network
>>>> entities.
>>>>
>>>> The sub-collection in the context of a Cluster is used to
>>>> attach/detach a
>>>> network from the DC of that cluster.
>>>> The network in the context of a cluster has some additional
>>>> information,
>>>> let's
>>>> say for example 'status' of the network:
>>>> If a network is defined on all hosts in the cluster then
>>>> it's
>>>> status is
>>>> 'Operational'.
>>>> If a network is not defined on some of the hosts in the
>>>> cluster
>>>> then
>>>> it's
>>>> status is 'Not Operational'[1].
>>>>
>>>>
>>>> -- Problem --
>>>> The problem is that details which are only relevant in context of
>>>> a
>>>> cluster, are still displayed in the root context as well (e.g:
>>>> 'status').
>>>> This can, in certain cases, cause unexpected behaviour.
>>>>
>>>> For example, let's consider this topology:
>>>> Data Center A
>>>> |
>>>> |\____ Network 'red'
>>>> |\____ Cluster A1
>>>> | \______ Network 'red' attached
>>>> \____ Cluster A2
>>>> \______ Network 'red' attached
>>>>
>>>> If the 'status' is the same on all the clusters that the network
>>>> is
>>>> attached to
>>>> (A1, A2) then there will be one element in the top level
>>>> collection,
>>>> with the
>>>> network details and the 'status' field representing the state
>>>> (which
>>>> is same
>>>> for all networks in the cluster contexts of the cluster).
>>>> If, however, the status is not the same (ie. on A1 the network is
>>>> 'Operational' and on A2 it is 'Non Operational') then
the
>>>> top-level
>>>> collection will show two elements for the network, where all
>>>> network
>>>> details are the same and only the 'status' field is different.
>>>>
>>>
>>> That sounds like a bug to me.
>>> I think that top collection should include only DC level
>>> properties and
>>> not cluster level properties, status should not be there (same as
>>> display required etc.)
>>>
>>>
>>>> This is problematic IMHO for several reasons:
>>>> 1. Showing one network in certain states, and multiple copies
>>>> of this
>>>> network in other states is not optimal, to say the least.
>>>> 2. In the top-level collection there is no indicator of the
>>>> cluster
>>>> for which
>>>> the network is displayed, so there is no way to
>>>> differentiate
>>>> between the
>>>> two 'red' network elements (they will have same id,
name,
>>>> etc.).
>>>> 3. There is a certain asymmetry between the remove action[2]
>>>> and the
>>>> result in that you would expect: you either remove a
>>>> network but
>>>> in the
>>>> result you would see several elements removed.
>>>>
>>>>
>>>> -- Proposed Solutions --
>>>> Personally I can think of several solutions to this problem:
>>>> 1. Declare the top-level collection as a collection of all
>>>> networks
>>>> that are
>>>> either attached to cluster or not, and if they are indeed
>>>> attached
>>>> then
>>>> show the details for each cluster, including a link to the
>>>> cluster.
>>>> 2. Declare the top-level collection as a collection of all
>>>> networks
>>>> that are
>>>> defined in data-centers, but they will not contain any
>>>> cluster
>>>> specific
>>>> data, and thus each entry is unique.
>>>>
>>>> Solution #2 is breaking the API backwards-compatibility, since it
>>>> includes
>>>> removing certain fields that have appeared today (namely
'status'
>>>> and
>>>> 'display') but IMO would give a better experience since the
>>>> top-level
>>>> collection is actually used for managing networks, and not their
>>>> attachment
>>>> to clusters which should be done in the context of each cluster.
>>>>
>>> I really don't think top collections should include cluster
>>> networks it
>>> is not user-friendly to say the least.
>>
>> how is that different from top collections including VMs and
>> templates?
>> (or logical networks becoming main tab in the UI going forward)
>>
>
> I think you missed the point of cluster network entity vs. DC network
> entity.
>
> we should have in the top collection a DC network, I think we should
> not
> display the network per cluster in top network collection (that can
> be
> viewed under the cluster context).
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?
11:49 a.m.
----- Original Message -----
From: "Itamar Heim" <iheim(a)redhat.com>
To: "Simon Grinberg" <simon(a)redhat.com>
Cc: "engine-devel" <engine-devel(a)ovirt.org>
Sent: Wednesday, July 4, 2012 7:38:46 PM
Subject: Re: [Engine-devel] Fwd: Problem in REST API handling/displaying of logical
networks
On 07/04/2012 07:27 PM, Simon Grinberg wrote:
>
>
> ----- Original Message -----
>> From: "Livnat Peer" <lpeer(a)redhat.com>
>> To: "Itamar Heim" <iheim(a)redhat.com>
>> Cc: "engine-devel" <engine-devel(a)ovirt.org>
>> Sent: Tuesday, July 3, 2012 10:06:37 PM
>> Subject: Re: [Engine-devel] Fwd: Problem in REST API
>> handling/displaying of logical networks
>>
>> On 03/07/12 21:28, Itamar Heim wrote:
>>> On 07/03/2012 10:30 AM, Livnat Peer wrote:
>>>> On 02/07/12 17:35, Mike Kolesnik wrote:
>>>>> Hi All,
>>>>>
>>>>> I would like to hear opinions about a behaviour that I think is
>>>>> problematic in
>>>>> REST API handling of logical networks.
>>>>>
>>>>> -- Intro --
>>>>> Today in the REST API we are exposing two collections for
>>>>> "logical
>>>>> network" related entities.
>>>>>
>>>>> First is a top level collection which is out of any context at
>>>>> the
>>>>> address
>>>>> http://engine/api/networks.
>>>>> Second is a sub-collection in the context of a cluster:
>>>>> http://engine/api/cluster/xxx/networks
>>>>>
>>>>> The network itself is defined per DC level, so for each DC you
>>>>> would
>>>>> have
>>>>> at least one logical network for management, which has some
>>>>> properties such
>>>>> as STP, MTU, etc..
>>>>> The top level collection is used to create/delete such network
>>>>> entities.
>>>>>
>>>>> The sub-collection in the context of a Cluster is used to
>>>>> attach/detach a
>>>>> network from the DC of that cluster.
>>>>> The network in the context of a cluster has some additional
>>>>> information,
>>>>> let's
>>>>> say for example 'status' of the network:
>>>>> If a network is defined on all hosts in the cluster then
>>>>> it's
>>>>> status is
>>>>> 'Operational'.
>>>>> If a network is not defined on some of the hosts in the
>>>>> cluster
>>>>> then
>>>>> it's
>>>>> status is 'Not Operational'[1].
>>>>>
>>>>>
>>>>> -- Problem --
>>>>> The problem is that details which are only relevant in context
>>>>> of
>>>>> a
>>>>> cluster, are still displayed in the root context as well (e.g:
>>>>> 'status').
>>>>> This can, in certain cases, cause unexpected behaviour.
>>>>>
>>>>> For example, let's consider this topology:
>>>>> Data Center A
>>>>> |
>>>>> |\____ Network 'red'
>>>>> |\____ Cluster A1
>>>>> | \______ Network 'red' attached
>>>>> \____ Cluster A2
>>>>> \______ Network 'red' attached
>>>>>
>>>>> If the 'status' is the same on all the clusters that the
>>>>> network
>>>>> is
>>>>> attached to
>>>>> (A1, A2) then there will be one element in the top level
>>>>> collection,
>>>>> with the
>>>>> network details and the 'status' field representing the
state
>>>>> (which
>>>>> is same
>>>>> for all networks in the cluster contexts of the cluster).
>>>>> If, however, the status is not the same (ie. on A1 the network
>>>>> is
>>>>> 'Operational' and on A2 it is 'Non Operational')
then the
>>>>> top-level
>>>>> collection will show two elements for the network, where all
>>>>> network
>>>>> details are the same and only the 'status' field is
different.
>>>>>
>>>>
>>>> That sounds like a bug to me.
>>>> I think that top collection should include only DC level
>>>> properties and
>>>> not cluster level properties, status should not be there (same
>>>> as
>>>> display required etc.)
>>>>
>>>>
>>>>> This is problematic IMHO for several reasons:
>>>>> 1. Showing one network in certain states, and multiple
>>>>> copies
>>>>> of this
>>>>> network in other states is not optimal, to say the
>>>>> least.
>>>>> 2. In the top-level collection there is no indicator of the
>>>>> cluster
>>>>> for which
>>>>> the network is displayed, so there is no way to
>>>>> differentiate
>>>>> between the
>>>>> two 'red' network elements (they will have same id,
>>>>> name,
>>>>> etc.).
>>>>> 3. There is a certain asymmetry between the remove
>>>>> action[2]
>>>>> and the
>>>>> result in that you would expect: you either remove a
>>>>> network but
>>>>> in the
>>>>> result you would see several elements removed.
>>>>>
>>>>>
>>>>> -- Proposed Solutions --
>>>>> Personally I can think of several solutions to this problem:
>>>>> 1. Declare the top-level collection as a collection of all
>>>>> networks
>>>>> that are
>>>>> either attached to cluster or not, and if they are
>>>>> indeed
>>>>> attached
>>>>> then
>>>>> show the details for each cluster, including a link to
>>>>> the
>>>>> cluster.
>>>>> 2. Declare the top-level collection as a collection of all
>>>>> networks
>>>>> that are
>>>>> defined in data-centers, but they will not contain any
>>>>> cluster
>>>>> specific
>>>>> data, and thus each entry is unique.
>>>>>
>>>>> Solution #2 is breaking the API backwards-compatibility, since
>>>>> it
>>>>> includes
>>>>> removing certain fields that have appeared today (namely
>>>>> 'status'
>>>>> and
>>>>> 'display') but IMO would give a better experience since the
>>>>> top-level
>>>>> collection is actually used for managing networks, and not
>>>>> their
>>>>> attachment
>>>>> to clusters which should be done in the context of each
>>>>> cluster.
>>>>>
>>>> I really don't think top collections should include cluster
>>>> networks it
>>>> is not user-friendly to say the least.
>>>
>>> how is that different from top collections including VMs and
>>> templates?
>>> (or logical networks becoming main tab in the UI going forward)
>>>
>>
>> I think you missed the point of cluster network entity vs. DC
>> network
>> entity.
>>
>> we should have in the top collection a DC network, I think we
>> should
>> not
>> display the network per cluster in top network collection (that
>> can
>> be
>> viewed under the cluster context).
>
> 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?
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
12:58 a.m.
On 04/07/12 19:49, Simon Grinberg wrote:
----- Original Message -----
> From: "Itamar Heim" <iheim(a)redhat.com>
> To: "Simon Grinberg" <simon(a)redhat.com>
> Cc: "engine-devel" <engine-devel(a)ovirt.org>
> Sent: Wednesday, July 4, 2012 7:38:46 PM
> Subject: Re: [Engine-devel] Fwd: Problem in REST API handling/displaying of logical
networks
>
> On 07/04/2012 07:27 PM, Simon Grinberg wrote:
>>
>>
>> ----- Original Message -----
>>> From: "Livnat Peer" <lpeer(a)redhat.com>
>>> To: "Itamar Heim" <iheim(a)redhat.com>
>>> Cc: "engine-devel" <engine-devel(a)ovirt.org>
>>> Sent: Tuesday, July 3, 2012 10:06:37 PM
>>> Subject: Re: [Engine-devel] Fwd: Problem in REST API
>>> handling/displaying of logical networks
>>>
>>> On 03/07/12 21:28, Itamar Heim wrote:
>>>> On 07/03/2012 10:30 AM, Livnat Peer wrote:
>>>>> On 02/07/12 17:35, Mike Kolesnik wrote:
>>>>>> Hi All,
>>>>>>
>>>>>> I would like to hear opinions about a behaviour that I think is
>>>>>> problematic in
>>>>>> REST API handling of logical networks.
>>>>>>
>>>>>> -- Intro --
>>>>>> Today in the REST API we are exposing two collections for
>>>>>> "logical
>>>>>> network" related entities.
>>>>>>
>>>>>> First is a top level collection which is out of any context at
>>>>>> the
>>>>>> address
>>>>>> http://engine/api/networks.
>>>>>> Second is a sub-collection in the context of a cluster:
>>>>>> http://engine/api/cluster/xxx/networks
>>>>>>
>>>>>> The network itself is defined per DC level, so for each DC you
>>>>>> would
>>>>>> have
>>>>>> at least one logical network for management, which has some
>>>>>> properties such
>>>>>> as STP, MTU, etc..
>>>>>> The top level collection is used to create/delete such network
>>>>>> entities.
>>>>>>
>>>>>> The sub-collection in the context of a Cluster is used to
>>>>>> attach/detach a
>>>>>> network from the DC of that cluster.
>>>>>> The network in the context of a cluster has some additional
>>>>>> information,
>>>>>> let's
>>>>>> say for example 'status' of the network:
>>>>>> If a network is defined on all hosts in the cluster then
>>>>>> it's
>>>>>> status is
>>>>>> 'Operational'.
>>>>>> If a network is not defined on some of the hosts in the
>>>>>> cluster
>>>>>> then
>>>>>> it's
>>>>>> status is 'Not Operational'[1].
>>>>>>
>>>>>>
>>>>>> -- Problem --
>>>>>> The problem is that details which are only relevant in context
>>>>>> of
>>>>>> a
>>>>>> cluster, are still displayed in the root context as well (e.g:
>>>>>> 'status').
>>>>>> This can, in certain cases, cause unexpected behaviour.
>>>>>>
>>>>>> For example, let's consider this topology:
>>>>>> Data Center A
>>>>>> |
>>>>>> |\____ Network 'red'
>>>>>> |\____ Cluster A1
>>>>>> | \______ Network 'red' attached
>>>>>> \____ Cluster A2
>>>>>> \______ Network 'red' attached
>>>>>>
>>>>>> If the 'status' is the same on all the clusters that the
>>>>>> network
>>>>>> is
>>>>>> attached to
>>>>>> (A1, A2) then there will be one element in the top level
>>>>>> collection,
>>>>>> with the
>>>>>> network details and the 'status' field representing the
state
>>>>>> (which
>>>>>> is same
>>>>>> for all networks in the cluster contexts of the cluster).
>>>>>> If, however, the status is not the same (ie. on A1 the network
>>>>>> is
>>>>>> 'Operational' and on A2 it is 'Non Operational')
then the
>>>>>> top-level
>>>>>> collection will show two elements for the network, where all
>>>>>> network
>>>>>> details are the same and only the 'status' field is
different.
>>>>>>
>>>>>
>>>>> That sounds like a bug to me.
>>>>> I think that top collection should include only DC level
>>>>> properties and
>>>>> not cluster level properties, status should not be there (same
>>>>> as
>>>>> display required etc.)
>>>>>
>>>>>
>>>>>> This is problematic IMHO for several reasons:
>>>>>> 1. Showing one network in certain states, and multiple
>>>>>> copies
>>>>>> of this
>>>>>> network in other states is not optimal, to say the
>>>>>> least.
>>>>>> 2. In the top-level collection there is no indicator of the
>>>>>> cluster
>>>>>> for which
>>>>>> the network is displayed, so there is no way to
>>>>>> differentiate
>>>>>> between the
>>>>>> two 'red' network elements (they will have same
id,
>>>>>> name,
>>>>>> etc.).
>>>>>> 3. There is a certain asymmetry between the remove
>>>>>> action[2]
>>>>>> and the
>>>>>> result in that you would expect: you either remove a
>>>>>> network but
>>>>>> in the
>>>>>> result you would see several elements removed.
>>>>>>
>>>>>>
>>>>>> -- Proposed Solutions --
>>>>>> Personally I can think of several solutions to this problem:
>>>>>> 1. Declare the top-level collection as a collection of all
>>>>>> networks
>>>>>> that are
>>>>>> either attached to cluster or not, and if they are
>>>>>> indeed
>>>>>> attached
>>>>>> then
>>>>>> show the details for each cluster, including a link to
>>>>>> the
>>>>>> cluster.
>>>>>> 2. Declare the top-level collection as a collection of all
>>>>>> networks
>>>>>> that are
>>>>>> defined in data-centers, but they will not contain any
>>>>>> cluster
>>>>>> specific
>>>>>> data, and thus each entry is unique.
>>>>>>
>>>>>> Solution #2 is breaking the API backwards-compatibility, since
>>>>>> it
>>>>>> includes
>>>>>> removing certain fields that have appeared today (namely
>>>>>> 'status'
>>>>>> and
>>>>>> 'display') but IMO would give a better experience since
the
>>>>>> top-level
>>>>>> collection is actually used for managing networks, and not
>>>>>> their
>>>>>> attachment
>>>>>> to clusters which should be done in the context of each
>>>>>> cluster.
>>>>>>
>>>>> I really don't think top collections should include cluster
>>>>> networks it
>>>>> is not user-friendly to say the least.
>>>>
>>>> how is that different from top collections including VMs and
>>>> templates?
>>>> (or logical networks becoming main tab in the UI going forward)
>>>>
>>>
>>> I think you missed the point of cluster network entity vs. DC
>>> network
>>> entity.
>>>
>>> we should have in the top collection a DC network, I think we
>>> should
>>> not
>>> display the network per cluster in top network collection (that
>>> can
>>> be
>>> viewed under the cluster context).
>>
>> 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?
I agree with Simon, we should consider this as a bug and not go through
a deprecation process.
Example - we have 3 clusters with a 'red; network attached and for one
of the clusters the network is non-operational. On the top collection
we'll have two instances of the network one with status operational and
the other with non-operational....
A user can't use the above information and if he did it is most likely
wrong / a bug.
Specifically for networks we reviewed the rest API and the problem is
there only for the top level collection (for property 'status' and other
properties that were only added in 3.1 so we can remove them safely).
Livnat
> _______________________________________________
> Engine-devel mailing list
> Engine-devel(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
>
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
1:18 a.m.
----- Original Message -----
From: "Livnat Peer" <lpeer(a)redhat.com>
To: "Simon Grinberg" <simon(a)redhat.com>
Cc: "engine-devel" <engine-devel(a)ovirt.org>
Sent: Thursday, July 5, 2012 8:58:20 AM
Subject: Re: [Engine-devel] Fwd: Problem in REST API handling/displaying of logical
networks
On 04/07/12 19:49, Simon Grinberg wrote:
>
>
> ----- Original Message -----
>> From: "Itamar Heim" <iheim(a)redhat.com>
>> To: "Simon Grinberg" <simon(a)redhat.com>
>> Cc: "engine-devel" <engine-devel(a)ovirt.org>
>> Sent: Wednesday, July 4, 2012 7:38:46 PM
>> Subject: Re: [Engine-devel] Fwd: Problem in REST API handling/displaying of
logical networks
>>
>> On 07/04/2012 07:27 PM, Simon Grinberg wrote:
>>>
>>>
>>> ----- Original Message -----
>>>> From: "Livnat Peer" <lpeer(a)redhat.com>
>>>> To: "Itamar Heim" <iheim(a)redhat.com>
>>>> Cc: "engine-devel" <engine-devel(a)ovirt.org>
>>>> Sent: Tuesday, July 3, 2012 10:06:37 PM
>>>> Subject: Re: [Engine-devel] Fwd: Problem in REST API
>>>> handling/displaying of logical networks
>>>>
>>>> On 03/07/12 21:28, Itamar Heim wrote:
>>>>> On 07/03/2012 10:30 AM, Livnat Peer wrote:
>>>>>> On 02/07/12 17:35, Mike Kolesnik wrote:
>>>>>>> Hi All,
>>>>>>>
>>>>>>> I would like to hear opinions about a behaviour that I think
is
>>>>>>> problematic in
>>>>>>> REST API handling of logical networks.
>>>>>>>
>>>>>>> -- Intro --
>>>>>>> Today in the REST API we are exposing two collections for
>>>>>>> "logical
>>>>>>> network" related entities.
>>>>>>>
>>>>>>> First is a top level collection which is out of any context
at
>>>>>>> the
>>>>>>> address
>>>>>>> http://engine/api/networks.
>>>>>>> Second is a sub-collection in the context of a cluster:
>>>>>>> http://engine/api/cluster/xxx/networks
>>>>>>>
>>>>>>> The network itself is defined per DC level, so for each DC
you
>>>>>>> would
>>>>>>> have
>>>>>>> at least one logical network for management, which has some
>>>>>>> properties such
>>>>>>> as STP, MTU, etc..
>>>>>>> The top level collection is used to create/delete such
network
>>>>>>> entities.
>>>>>>>
>>>>>>> The sub-collection in the context of a Cluster is used to
>>>>>>> attach/detach a
>>>>>>> network from the DC of that cluster.
>>>>>>> The network in the context of a cluster has some additional
>>>>>>> information,
>>>>>>> let's
>>>>>>> say for example 'status' of the network:
>>>>>>> If a network is defined on all hosts in the cluster
then
>>>>>>> it's
>>>>>>> status is
>>>>>>> 'Operational'.
>>>>>>> If a network is not defined on some of the hosts in
the
>>>>>>> cluster
>>>>>>> then
>>>>>>> it's
>>>>>>> status is 'Not Operational'[1].
>>>>>>>
>>>>>>>
>>>>>>> -- Problem --
>>>>>>> The problem is that details which are only relevant in
context
>>>>>>> of
>>>>>>> a
>>>>>>> cluster, are still displayed in the root context as well
(e.g:
>>>>>>> 'status').
>>>>>>> This can, in certain cases, cause unexpected behaviour.
>>>>>>>
>>>>>>> For example, let's consider this topology:
>>>>>>> Data Center A
>>>>>>> |
>>>>>>> |\____ Network 'red'
>>>>>>> |\____ Cluster A1
>>>>>>> | \______ Network 'red'
attached
>>>>>>> \____ Cluster A2
>>>>>>> \______ Network 'red'
attached
>>>>>>>
>>>>>>> If the 'status' is the same on all the clusters that
the
>>>>>>> network
>>>>>>> is
>>>>>>> attached to
>>>>>>> (A1, A2) then there will be one element in the top level
>>>>>>> collection,
>>>>>>> with the
>>>>>>> network details and the 'status' field representing
the state
>>>>>>> (which
>>>>>>> is same
>>>>>>> for all networks in the cluster contexts of the cluster).
>>>>>>> If, however, the status is not the same (ie. on A1 the
network
>>>>>>> is
>>>>>>> 'Operational' and on A2 it is 'Non
Operational') then the
>>>>>>> top-level
>>>>>>> collection will show two elements for the network, where all
>>>>>>> network
>>>>>>> details are the same and only the 'status' field is
different.
>>>>>>>
>>>>>>
>>>>>> That sounds like a bug to me.
>>>>>> I think that top collection should include only DC level
>>>>>> properties and
>>>>>> not cluster level properties, status should not be there (same
>>>>>> as
>>>>>> display required etc.)
>>>>>>
>>>>>>
>>>>>>> This is problematic IMHO for several reasons:
>>>>>>> 1. Showing one network in certain states, and multiple
>>>>>>> copies
>>>>>>> of this
>>>>>>> network in other states is not optimal, to say the
>>>>>>> least.
>>>>>>> 2. In the top-level collection there is no indicator of
the
>>>>>>> cluster
>>>>>>> for which
>>>>>>> the network is displayed, so there is no way to
>>>>>>> differentiate
>>>>>>> between the
>>>>>>> two 'red' network elements (they will have
same id,
>>>>>>> name,
>>>>>>> etc.).
>>>>>>> 3. There is a certain asymmetry between the remove
>>>>>>> action[2]
>>>>>>> and the
>>>>>>> result in that you would expect: you either remove a
>>>>>>> network but
>>>>>>> in the
>>>>>>> result you would see several elements removed.
>>>>>>>
>>>>>>>
>>>>>>> -- Proposed Solutions --
>>>>>>> Personally I can think of several solutions to this problem:
>>>>>>> 1. Declare the top-level collection as a collection of
all
>>>>>>> networks
>>>>>>> that are
>>>>>>> either attached to cluster or not, and if they are
>>>>>>> indeed
>>>>>>> attached
>>>>>>> then
>>>>>>> show the details for each cluster, including a link
to
>>>>>>> the
>>>>>>> cluster.
>>>>>>> 2. Declare the top-level collection as a collection of
all
>>>>>>> networks
>>>>>>> that are
>>>>>>> defined in data-centers, but they will not contain
any
>>>>>>> cluster
>>>>>>> specific
>>>>>>> data, and thus each entry is unique.
>>>>>>>
>>>>>>> Solution #2 is breaking the API backwards-compatibility,
since
>>>>>>> it
>>>>>>> includes
>>>>>>> removing certain fields that have appeared today (namely
>>>>>>> 'status'
>>>>>>> and
>>>>>>> 'display') but IMO would give a better experience
since the
>>>>>>> top-level
>>>>>>> collection is actually used for managing networks, and not
>>>>>>> their
>>>>>>> attachment
>>>>>>> to clusters which should be done in the context of each
>>>>>>> cluster.
>>>>>>>
>>>>>> I really don't think top collections should include cluster
>>>>>> networks it
>>>>>> is not user-friendly to say the least.
>>>>>
>>>>> how is that different from top collections including VMs and
>>>>> templates?
>>>>> (or logical networks becoming main tab in the UI going forward)
>>>>>
>>>>
>>>> I think you missed the point of cluster network entity vs. DC
>>>> network
>>>> entity.
>>>>
>>>> we should have in the top collection a DC network, I think we
>>>> should
>>>> not
>>>> display the network per cluster in top network collection (that
>>>> can
>>>> be
>>>> viewed under the cluster context).
>>>
>>> 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?
>
I agree with Simon, we should consider this as a bug and not go through
a deprecation process.
Example - we have 3 clusters with a 'red; network attached and for one
of the clusters the network is non-operational. On the top collection
we'll have two instances of the network one with status operational and
the other with non-operational....
A user can't use the above information and if he did it is most likely
wrong / a bug.
+1
I also think it's broken enough to be seen as a bug; it's hard to imagine
a useful script which uses 'status' field at root level.
Specifically for networks we reviewed the rest API and the problem is
there only for the top level collection (for property 'status' and other
properties that were only added in 3.1 so we can remove them safely).
Livnat
>
>
>
>> _______________________________________________
>> Engine-devel mailing list
>> Engine-devel(a)ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/engine-devel
>>
> _______________________________________________
> Engine-devel mailing list
> Engine-devel(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
>
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
2:24 a.m.
New subject: [Engine-devel] Fwd: Problem in REST API handling/displaying of logical networks
----- Original Message -----
----- Original Message -----
> From: "Itamar Heim" <iheim(a)redhat.com>
> To: "Simon Grinberg" <simon(a)redhat.com>
> Cc: "engine-devel" <engine-devel(a)ovirt.org>
> Sent: Wednesday, July 4, 2012 7:38:46 PM
> Subject: Re: [Engine-devel] Fwd: Problem in REST API
> handling/displaying of logical networks
>
> On 07/04/2012 07:27 PM, Simon Grinberg wrote:
> >
> >
> > ----- Original Message -----
> >> From: "Livnat Peer" <lpeer(a)redhat.com>
> >> To: "Itamar Heim" <iheim(a)redhat.com>
> >> Cc: "engine-devel" <engine-devel(a)ovirt.org>
> >> Sent: Tuesday, July 3, 2012 10:06:37 PM
> >> Subject: Re: [Engine-devel] Fwd: Problem in REST API
> >> handling/displaying of logical networks
> >>
> >> On 03/07/12 21:28, Itamar Heim wrote:
> >>> On 07/03/2012 10:30 AM, Livnat Peer wrote:
> >>>> On 02/07/12 17:35, Mike Kolesnik wrote:
> >>>>> Hi All,
> >>>>>
> >>>>> I would like to hear opinions about a behaviour that I think
> >>>>> is
> >>>>> problematic in
> >>>>> REST API handling of logical networks.
> >>>>>
> >>>>> -- Intro --
> >>>>> Today in the REST API we are exposing two collections for
> >>>>> "logical
> >>>>> network" related entities.
> >>>>>
> >>>>> First is a top level collection which is out of any context
> >>>>> at
> >>>>> the
> >>>>> address
> >>>>> http://engine/api/networks.
> >>>>> Second is a sub-collection in the context of a cluster:
> >>>>> http://engine/api/cluster/xxx/networks
> >>>>>
> >>>>> The network itself is defined per DC level, so for each DC
> >>>>> you
> >>>>> would
> >>>>> have
> >>>>> at least one logical network for management, which has some
> >>>>> properties such
> >>>>> as STP, MTU, etc..
> >>>>> The top level collection is used to create/delete such
> >>>>> network
> >>>>> entities.
> >>>>>
> >>>>> The sub-collection in the context of a Cluster is used to
> >>>>> attach/detach a
> >>>>> network from the DC of that cluster.
> >>>>> The network in the context of a cluster has some additional
> >>>>> information,
> >>>>> let's
> >>>>> say for example 'status' of the network:
> >>>>> If a network is defined on all hosts in the cluster
> >>>>> then
> >>>>> it's
> >>>>> status is
> >>>>> 'Operational'.
> >>>>> If a network is not defined on some of the hosts in the
> >>>>> cluster
> >>>>> then
> >>>>> it's
> >>>>> status is 'Not Operational'[1].
> >>>>>
> >>>>>
> >>>>> -- Problem --
> >>>>> The problem is that details which are only relevant in
> >>>>> context
> >>>>> of
> >>>>> a
> >>>>> cluster, are still displayed in the root context as well
> >>>>> (e.g:
> >>>>> 'status').
> >>>>> This can, in certain cases, cause unexpected behaviour.
> >>>>>
> >>>>> For example, let's consider this topology:
> >>>>> Data Center A
> >>>>> |
> >>>>> |\____ Network 'red'
> >>>>> |\____ Cluster A1
> >>>>> | \______ Network 'red'
attached
> >>>>> \____ Cluster A2
> >>>>> \______ Network 'red'
attached
> >>>>>
> >>>>> If the 'status' is the same on all the clusters that
the
> >>>>> network
> >>>>> is
> >>>>> attached to
> >>>>> (A1, A2) then there will be one element in the top level
> >>>>> collection,
> >>>>> with the
> >>>>> network details and the 'status' field representing the
state
> >>>>> (which
> >>>>> is same
> >>>>> for all networks in the cluster contexts of the cluster).
> >>>>> If, however, the status is not the same (ie. on A1 the
> >>>>> network
> >>>>> is
> >>>>> 'Operational' and on A2 it is 'Non
Operational') then the
> >>>>> top-level
> >>>>> collection will show two elements for the network, where all
> >>>>> network
> >>>>> details are the same and only the 'status' field is
> >>>>> different.
> >>>>>
> >>>>
> >>>> That sounds like a bug to me.
> >>>> I think that top collection should include only DC level
> >>>> properties and
> >>>> not cluster level properties, status should not be there (same
> >>>> as
> >>>> display required etc.)
> >>>>
> >>>>
> >>>>> This is problematic IMHO for several reasons:
> >>>>> 1. Showing one network in certain states, and multiple
> >>>>> copies
> >>>>> of this
> >>>>> network in other states is not optimal, to say the
> >>>>> least.
> >>>>> 2. In the top-level collection there is no indicator of
> >>>>> the
> >>>>> cluster
> >>>>> for which
> >>>>> the network is displayed, so there is no way to
> >>>>> differentiate
> >>>>> between the
> >>>>> two 'red' network elements (they will have same
id,
> >>>>> name,
> >>>>> etc.).
> >>>>> 3. There is a certain asymmetry between the remove
> >>>>> action[2]
> >>>>> and the
> >>>>> result in that you would expect: you either remove a
> >>>>> network but
> >>>>> in the
> >>>>> result you would see several elements removed.
> >>>>>
> >>>>>
> >>>>> -- Proposed Solutions --
> >>>>> Personally I can think of several solutions to this problem:
> >>>>> 1. Declare the top-level collection as a collection of
> >>>>> all
> >>>>> networks
> >>>>> that are
> >>>>> either attached to cluster or not, and if they are
> >>>>> indeed
> >>>>> attached
> >>>>> then
> >>>>> show the details for each cluster, including a link
> >>>>> to
> >>>>> the
> >>>>> cluster.
> >>>>> 2. Declare the top-level collection as a collection of
> >>>>> all
> >>>>> networks
> >>>>> that are
> >>>>> defined in data-centers, but they will not contain
> >>>>> any
> >>>>> cluster
> >>>>> specific
> >>>>> data, and thus each entry is unique.
> >>>>>
> >>>>> Solution #2 is breaking the API backwards-compatibility,
> >>>>> since
> >>>>> it
> >>>>> includes
> >>>>> removing certain fields that have appeared today (namely
> >>>>> 'status'
> >>>>> and
> >>>>> 'display') but IMO would give a better experience since
the
> >>>>> top-level
> >>>>> collection is actually used for managing networks, and not
> >>>>> their
> >>>>> attachment
> >>>>> to clusters which should be done in the context of each
> >>>>> cluster.
> >>>>>
> >>>> I really don't think top collections should include cluster
> >>>> networks it
> >>>> is not user-friendly to say the least.
> >>>
> >>> how is that different from top collections including VMs and
> >>> templates?
> >>> (or logical networks becoming main tab in the UI going forward)
> >>>
> >>
> >> I think you missed the point of cluster network entity vs. DC
> >> network
> >> entity.
> >>
> >> we should have in the top collection a DC network, I think we
> >> should
> >> not
> >> display the network per cluster in top network collection (that
> >> can
> >> be
> >> viewed under the cluster context).
> >
> > 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...
Any suggestions of how to do that?
> _______________________________________________
> Engine-devel mailing list
> Engine-devel(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
>
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
2:51 a.m.
On 05/07/12 10:24, Miki Kenneth wrote:
----- Original Message -----
>
>
> ----- Original Message -----
>> From: "Itamar Heim" <iheim(a)redhat.com>
>> To: "Simon Grinberg" <simon(a)redhat.com>
>> Cc: "engine-devel" <engine-devel(a)ovirt.org>
>> Sent: Wednesday, July 4, 2012 7:38:46 PM
>> Subject: Re: [Engine-devel] Fwd: Problem in REST API
>> handling/displaying of logical networks
>>
>> On 07/04/2012 07:27 PM, Simon Grinberg wrote:
>>>
>>>
>>> ----- Original Message -----
>>>> From: "Livnat Peer" <lpeer(a)redhat.com>
>>>> To: "Itamar Heim" <iheim(a)redhat.com>
>>>> Cc: "engine-devel" <engine-devel(a)ovirt.org>
>>>> Sent: Tuesday, July 3, 2012 10:06:37 PM
>>>> Subject: Re: [Engine-devel] Fwd: Problem in REST API
>>>> handling/displaying of logical networks
>>>>
>>>> On 03/07/12 21:28, Itamar Heim wrote:
>>>>> On 07/03/2012 10:30 AM, Livnat Peer wrote:
>>>>>> On 02/07/12 17:35, Mike Kolesnik wrote:
>>>>>>> Hi All,
>>>>>>>
>>>>>>> I would like to hear opinions about a behaviour that I think
>>>>>>> is
>>>>>>> problematic in
>>>>>>> REST API handling of logical networks.
>>>>>>>
>>>>>>> -- Intro --
>>>>>>> Today in the REST API we are exposing two collections for
>>>>>>> "logical
>>>>>>> network" related entities.
>>>>>>>
>>>>>>> First is a top level collection which is out of any context
>>>>>>> at
>>>>>>> the
>>>>>>> address
>>>>>>> http://engine/api/networks.
>>>>>>> Second is a sub-collection in the context of a cluster:
>>>>>>> http://engine/api/cluster/xxx/networks
>>>>>>>
>>>>>>> The network itself is defined per DC level, so for each DC
>>>>>>> you
>>>>>>> would
>>>>>>> have
>>>>>>> at least one logical network for management, which has some
>>>>>>> properties such
>>>>>>> as STP, MTU, etc..
>>>>>>> The top level collection is used to create/delete such
>>>>>>> network
>>>>>>> entities.
>>>>>>>
>>>>>>> The sub-collection in the context of a Cluster is used to
>>>>>>> attach/detach a
>>>>>>> network from the DC of that cluster.
>>>>>>> The network in the context of a cluster has some additional
>>>>>>> information,
>>>>>>> let's
>>>>>>> say for example 'status' of the network:
>>>>>>> If a network is defined on all hosts in the cluster
>>>>>>> then
>>>>>>> it's
>>>>>>> status is
>>>>>>> 'Operational'.
>>>>>>> If a network is not defined on some of the hosts in
the
>>>>>>> cluster
>>>>>>> then
>>>>>>> it's
>>>>>>> status is 'Not Operational'[1].
>>>>>>>
>>>>>>>
>>>>>>> -- Problem --
>>>>>>> The problem is that details which are only relevant in
>>>>>>> context
>>>>>>> of
>>>>>>> a
>>>>>>> cluster, are still displayed in the root context as well
>>>>>>> (e.g:
>>>>>>> 'status').
>>>>>>> This can, in certain cases, cause unexpected behaviour.
>>>>>>>
>>>>>>> For example, let's consider this topology:
>>>>>>> Data Center A
>>>>>>> |
>>>>>>> |\____ Network 'red'
>>>>>>> |\____ Cluster A1
>>>>>>> | \______ Network 'red'
attached
>>>>>>> \____ Cluster A2
>>>>>>> \______ Network 'red'
attached
>>>>>>>
>>>>>>> If the 'status' is the same on all the clusters that
the
>>>>>>> network
>>>>>>> is
>>>>>>> attached to
>>>>>>> (A1, A2) then there will be one element in the top level
>>>>>>> collection,
>>>>>>> with the
>>>>>>> network details and the 'status' field representing
the state
>>>>>>> (which
>>>>>>> is same
>>>>>>> for all networks in the cluster contexts of the cluster).
>>>>>>> If, however, the status is not the same (ie. on A1 the
>>>>>>> network
>>>>>>> is
>>>>>>> 'Operational' and on A2 it is 'Non
Operational') then the
>>>>>>> top-level
>>>>>>> collection will show two elements for the network, where all
>>>>>>> network
>>>>>>> details are the same and only the 'status' field is
>>>>>>> different.
>>>>>>>
>>>>>>
>>>>>> That sounds like a bug to me.
>>>>>> I think that top collection should include only DC level
>>>>>> properties and
>>>>>> not cluster level properties, status should not be there (same
>>>>>> as
>>>>>> display required etc.)
>>>>>>
>>>>>>
>>>>>>> This is problematic IMHO for several reasons:
>>>>>>> 1. Showing one network in certain states, and multiple
>>>>>>> copies
>>>>>>> of this
>>>>>>> network in other states is not optimal, to say the
>>>>>>> least.
>>>>>>> 2. In the top-level collection there is no indicator of
>>>>>>> the
>>>>>>> cluster
>>>>>>> for which
>>>>>>> the network is displayed, so there is no way to
>>>>>>> differentiate
>>>>>>> between the
>>>>>>> two 'red' network elements (they will have
same id,
>>>>>>> name,
>>>>>>> etc.).
>>>>>>> 3. There is a certain asymmetry between the remove
>>>>>>> action[2]
>>>>>>> and the
>>>>>>> result in that you would expect: you either remove a
>>>>>>> network but
>>>>>>> in the
>>>>>>> result you would see several elements removed.
>>>>>>>
>>>>>>>
>>>>>>> -- Proposed Solutions --
>>>>>>> Personally I can think of several solutions to this problem:
>>>>>>> 1. Declare the top-level collection as a collection of
>>>>>>> all
>>>>>>> networks
>>>>>>> that are
>>>>>>> either attached to cluster or not, and if they are
>>>>>>> indeed
>>>>>>> attached
>>>>>>> then
>>>>>>> show the details for each cluster, including a link
>>>>>>> to
>>>>>>> the
>>>>>>> cluster.
>>>>>>> 2. Declare the top-level collection as a collection of
>>>>>>> all
>>>>>>> networks
>>>>>>> that are
>>>>>>> defined in data-centers, but they will not contain
>>>>>>> any
>>>>>>> cluster
>>>>>>> specific
>>>>>>> data, and thus each entry is unique.
>>>>>>>
>>>>>>> Solution #2 is breaking the API backwards-compatibility,
>>>>>>> since
>>>>>>> it
>>>>>>> includes
>>>>>>> removing certain fields that have appeared today (namely
>>>>>>> 'status'
>>>>>>> and
>>>>>>> 'display') but IMO would give a better experience
since the
>>>>>>> top-level
>>>>>>> collection is actually used for managing networks, and not
>>>>>>> their
>>>>>>> attachment
>>>>>>> to clusters which should be done in the context of each
>>>>>>> cluster.
>>>>>>>
>>>>>> I really don't think top collections should include cluster
>>>>>> networks it
>>>>>> is not user-friendly to say the least.
>>>>>
>>>>> how is that different from top collections including VMs and
>>>>> templates?
>>>>> (or logical networks becoming main tab in the UI going forward)
>>>>>
>>>>
>>>> I think you missed the point of cluster network entity vs. DC
>>>> network
>>>> entity.
>>>>
>>>> we should have in the top collection a DC network, I think we
>>>> should
>>>> not
>>>> display the network per cluster in top network collection (that
>>>> can
>>>> be
>>>> viewed under the cluster context).
>>>
>>> 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.
Can you please open a bug / update the existing bug to reflect that.
Thanks, Livnat
Any suggestions of how to do that?
>
>
>
>> _______________________________________________
>> Engine-devel mailing list
>> Engine-devel(a)ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/engine-devel
>>
> _______________________________________________
> Engine-devel mailing list
> Engine-devel(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
>
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
3:13 a.m.
----- Original Message -----
On 05/07/12 10:24, Miki Kenneth wrote:
>
>
> ----- Original Message -----
>>
>>
>> ----- Original Message -----
>>> From: "Itamar Heim" <iheim(a)redhat.com>
>>> To: "Simon Grinberg" <simon(a)redhat.com>
>>> Cc: "engine-devel" <engine-devel(a)ovirt.org>
>>> Sent: Wednesday, July 4, 2012 7:38:46 PM
>>> Subject: Re: [Engine-devel] Fwd: Problem in REST API
>>> handling/displaying of logical networks
>>>
>>> On 07/04/2012 07:27 PM, Simon Grinberg 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.
Can you please open a bug / update the existing bug to reflect that.
OK then I will review the relevant bug(s) and update as needed.
Thank you all for the feedback!
Thanks,
Mike
Thanks, Livnat
> Any suggestions of how to do that?
3:31 a.m.
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)
Can you please open a bug / update the existing bug to reflect that.
Thanks, Livnat
--
Michael Pasternak
RedHat, ENG-Virtualization R&D
3:40 a.m.
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.
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.
we can always add it later and it does not change the fact that the API
changes.
> Can you please open a bug / update the existing bug to reflect that.
>
> Thanks, Livnat
>
>
>
3:56 a.m.
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.
(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
>>
>>
>>
>
>
--
Michael Pasternak
RedHat, ENG-Virtualization R&D
4 a.m.
----- Original Message -----
From: "Michael Pasternak" <mpastern(a)redhat.com>
To: "Livnat Peer" <lpeer(a)redhat.com>
Cc: "engine-devel" <engine-devel(a)ovirt.org>, "Simon Grinberg"
<simon(a)redhat.com>
Sent: Thursday, July 5, 2012 11:31:41 AM
Subject: Re: [Engine-devel] Fwd: Problem in REST API handling/displaying of logical
networks
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)
With storage domains attached/unattached is generally a 1:1 so it may make sense in a
way.
* not sure it's going to be in the future with shared read only export domain
* It's probably wrong even today with ISO domain in case that the setup contains more
then one DC.
With Networks it fore may be attached to partial collection on clusters, de facto that
will only say it is in uses by at least one cluster.
So in both cases this is wrong,
If you insist on maintaining this property the only valid values that I can see ATM is
INUSE vs UNUSED. - This should be true both for storage domains and logical networks.
> Can you please open a bug / update the existing bug to reflect
> that.
>
> Thanks, Livnat
>
>
>
--
Michael Pasternak
RedHat, ENG-Virtualization R&D
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
4:06 a.m.
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
>>>
>>>
>>>
>>
>>
>
>
4:06 a.m.
----- Original Message -----
From: "Michael Pasternak" <mpastern(a)redhat.com>
To: "Livnat Peer" <lpeer(a)redhat.com>
Cc: "engine-devel" <engine-devel(a)ovirt.org>, "Simon Grinberg"
<simon(a)redhat.com>
Sent: Thursday, July 5, 2012 11:56:03 AM
Subject: Re: [Engine-devel] Fwd: Problem in REST API handling/displaying of logical
networks
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.
(without this <status> you'll have to traverse over all networks
against all
clusters to find one unused)
Previous email sent in delay due to network disconnection,
So we are saying the same, that the status if left de facto indicates used/unused so why
not to use the proper terminology? attached is already an over loaded term
>
> 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
>>>
>>>
>>>
>>
>>
>
>
--
Michael Pasternak
RedHat, ENG-Virtualization R&D
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
4:09 a.m.
On 07/05/2012 12:00 PM, Simon Grinberg wrote:
----- Original Message -----
> From: "Michael Pasternak" <mpastern(a)redhat.com>
> To: "Livnat Peer" <lpeer(a)redhat.com>
> Cc: "engine-devel" <engine-devel(a)ovirt.org>, "Simon
Grinberg" <simon(a)redhat.com>
> Sent: Thursday, July 5, 2012 11:31:41 AM
> Subject: Re: [Engine-devel] Fwd: Problem in REST API handling/displaying of logical
networks
>
> 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)
With storage domains attached/unattached is generally a 1:1 so it may make sense in a
way.
* not sure it's going to be in the future with shared read only export domain
* It's probably wrong even today with ISO domain in case that the setup contains more
then one DC.
attached|unattached does not imply on amount of resources it being used in,
only used/not-used
With Networks it fore may be attached to partial collection on clusters, de facto that
will only say it is in uses by at least one cluster.
that's the purpose, see my other email (to Livnat) on this.
So in both cases this is wrong,
why? i'd say it's exactly serve the goal.
If you insist on maintaining this property the only valid values that I can see ATM is
INUSE vs UNUSED. - This should be true both for storage domains and logical networks.
i don't mind, only problem is that SD uses UNATTACHED and you told by yourself
that it's 1:1, so i guess we won't break SD api for consistency ...
>
>> Can you please open a bug / update the existing bug to reflect
>> that.
>>
>> Thanks, Livnat
>>
>>
>>
>
>
> --
>
> Michael Pasternak
> RedHat, ENG-Virtualization R&D
> _______________________________________________
> Engine-devel mailing list
> Engine-devel(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
>
--
Michael Pasternak
RedHat, ENG-Virtualization R&D
4:13 a.m.
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 ....
> (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
>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>
>
--
Michael Pasternak
RedHat, ENG-Virtualization R&D
4:19 a.m.
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
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>
>
4:32 a.m.
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
5:23 a.m.
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.
Please comment what option seems valid (I though we were going to the direction of fix
#2).
Thanks,
Mike
5:38 a.m.
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?
Please comment what option seems valid (I though we were going to the
direction of fix #2).
Thanks,
Mike
5:43 a.m.
----- 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.
> Please comment what option seems valid (I though we were going to
> the direction of fix #2).
>
> Thanks,
> Mike
>
5:50 a.m.
On 07/05/2012 01:38 PM, Livnat Peer wrote:
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?
+1 for #2. It will simplify the presentation of the networks on DC level.
> 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(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
5:53 a.m.
New subject: [Engine-devel] Fwd: Problem in REST API handling/displaying of logical networks
----- Original Message -----
From: "Mike Kolesnik" <mkolesni(a)redhat.com>
To: "engine-devel" <engine-devel(a)ovirt.org>
Cc: "Simon Grinberg" <simon(a)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(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
6:26 a.m.
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
6:38 a.m.
-----Original Message-----
From: engine-devel-bounces(a)ovirt.org
[mailto:engine-devel-bounces@ovirt.org]
On Behalf Of Livnat Peer
Sent: Thursday, July 05, 2012 1:39 PM
To: Mike Kolesnik
Cc: engine-devel; Simon Grinberg
Subject: Re: [Engine-devel] Fwd: Problem in REST API handling/displaying
of
logical networks
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?
It seems like #2 is more reasonable. +1 for #2
> 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(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
6:57 a.m.
On 07/05/2012 02:26 PM, Michael Pasternak wrote:
+1 for #2 (but only for <display> and new 3.1 props),
-2 for removing <status> (based on told above)
agreed with Livnat that we may implement collection of links
to clusters given network attached to instead of <status> field.
+1
--
Michael Pasternak
RedHat, ENG-Virtualization R&D
4303
days inactive
4309
days old
36 comments
12 participants
participants (12)
-
Avi Tal -
Igor Lvovsky -
Itamar Heim -
Livnat Peer -
Michael Pasternak -
Mike Kolesnik -
Miki Kenneth -
Moti Asayag -
Ori Liel -
Oved Ourfalli -
Simon Grinberg -
Yaniv Kaul