Michael Pasternak has posted comments on this change.

Change subject: restapi: Rest API for brick advanced details
......................................................................


Patch Set 3: I would prefer that you didn't submit this

(8 inline comments)

mpastern> in api we have /statistics concept, most of your patch should be 
implemented via /bricks/xxx/statistics/yyy
and not as brick properties,
<mpastern> i.e bytes_read, bytes_written, memory stats, etc
<mpastern> just see /vms/xxx/statistics
<mpastern> or /disks/xxx/statistics
<mpastern> or nics

....................................................
File 
backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd
Line 2924:     <xs:sequence>
Line 2925:       <xs:element name="host_name" type="xs:string" minOccurs="0" 
maxOccurs="1"/>
Line 2926:       <xs:element name="client_port" type="xs:int" minOccurs="0" 
maxOccurs="1"/>
Line 2927:       <xs:element name="bytes_read" type="xs:int" minOccurs="0" 
maxOccurs="1"/>
Line 2928:       <xs:element name="bytes_written" type="xs:int" minOccurs="0" 
maxOccurs="1"/>
in api we have statistics concept, bytes_read, bytes_written is a good candidate
for bricks/xxx/statistics sub-collection, see /vms/xxx/statistics for more 
details.
Line 2929:     </xs:sequence>
Line 2930:   </xs:complexType>
Line 2931: 
Line 2932:   <xs:element name="gluster_clients" type="GlusterClients"/>


Line 2944:     </xs:extension>
Line 2945:     </xs:complexContent>
Line 2946:   </xs:complexType>
Line 2947: 
Line 2948:   <xs:element name="gluster_memory_pool" type="GlusterMemoryPool"/>
we trying to make only tow period based names in api, can you change it to 
"memory_pool"?
Line 2949:   <xs:complexType name="GlusterMemoryPool">
Line 2950:     <xs:sequence>
Line 2951:       <xs:element name="name" type="xs:string" minOccurs="0" 
maxOccurs="1"/>
Line 2952:       <xs:element name="hot_count" type="xs:int" minOccurs="0" 
maxOccurs="1"/>


Line 2954:       <xs:element name="padded_size" type="xs:int" minOccurs="0" 
maxOccurs="1"/>
Line 2955:       <xs:element name="alloc_count" type="xs:int" minOccurs="0" 
maxOccurs="1"/>
Line 2956:       <xs:element name="max_alloc" type="xs:int" minOccurs="0" 
maxOccurs="1"/>
Line 2957:       <xs:element name="pool_misses" type="xs:int" minOccurs="0" 
maxOccurs="1"/>
Line 2958:       <xs:element name="max_std_alloc" type="xs:int" minOccurs="0" 
maxOccurs="1"/>
entire GlusterMemoryPool sounds as a part of statistics
Line 2959:     </xs:sequence>
Line 2960:   </xs:complexType>
Line 2961: 
Line 2962:   <xs:element name="gluster_memory_pools" type="GlusterMemoryPools"/>


Line 2958:       <xs:element name="max_std_alloc" type="xs:int" minOccurs="0" 
maxOccurs="1"/>
Line 2959:     </xs:sequence>
Line 2960:   </xs:complexType>
Line 2961: 
Line 2962:   <xs:element name="gluster_memory_pools" type="GlusterMemoryPools"/>
"memory_pools"?
Line 2963:   <xs:complexType name="GlusterMemoryPools">
Line 2964:     <xs:complexContent>
Line 2965:       <xs:extension base="BaseResources">
Line 2966:         <xs:sequence>


Line 2974:       </xs:extension>
Line 2975:     </xs:complexContent>
Line 2976:   </xs:complexType>
Line 2977: 
Line 2978:   <xs:element name="gluster_brick_memory_info" 
type="GlusterBrickMemoryInfo"/>
memory_info?
Line 2979:   <xs:complexType name="GlusterBrickMemoryInfo">
Line 2980:     <xs:sequence>
Line 2981:       <!-- equivalent to arena -->
Line 2982:       <xs:element name="total_alloc" type="xs:int" minOccurs="0" 
maxOccurs="1"/>


Line 2997:       <!-- equivalent to fordblks -->
Line 2998:       <xs:element name="total_free_space" type="xs:int" 
minOccurs="0" maxOccurs="1"/>
Line 2999:       <!-- equivalent to keepcost -->
Line 3000:       <xs:element name="releasable_free_space" type="xs:int" 
minOccurs="0" maxOccurs="1"/>
Line 3001:       <xs:element ref="gluster_memory_pools" minOccurs="0" 
maxOccurs="1"/>
memory_pools?

explanations for need of this change can be seen in "memory_info" bellow.
Line 3002:     </xs:sequence>
Line 3003:   </xs:complexType>
Line 3004: 
Line 3005:   <xs:element name="brick_details" 
type="GlusterBrickAdvancedDetails" />


Line 3019:           <xs:element name="device" type="xs:string" minOccurs="0" 
maxOccurs="1"/>
Line 3020:           <xs:element name="block_size" type="xs:int" minOccurs="0" 
maxOccurs="1"/>
Line 3021:           <xs:element name="mnt_options" type="xs:string" 
minOccurs="0" maxOccurs="1"/>
Line 3022:           <xs:element name="fs_name" type="xs:string" minOccurs="0" 
maxOccurs="1"/>
Line 3023:           <xs:element name="clients" type="GlusterClients" 
minOccurs="0" maxOccurs="1" />
two options here:


1. call this property gluster_clients

or

2. rename element to 

<xs:element name="clients" type="GlusterClients"/>,

explanations for need of this change can be seen in "memory_info" bellow.
Line 3024:           <xs:element name="memory_info" 
type="GlusterBrickMemoryInfo" minOccurs="0" maxOccurs="1"/>
Line 3025:         </xs:sequence>
Line 3026:       </xs:extension>
Line 3027:     </xs:complexContent>


Line 3020:           <xs:element name="block_size" type="xs:int" minOccurs="0" 
maxOccurs="1"/>
Line 3021:           <xs:element name="mnt_options" type="xs:string" 
minOccurs="0" maxOccurs="1"/>
Line 3022:           <xs:element name="fs_name" type="xs:string" minOccurs="0" 
maxOccurs="1"/>
Line 3023:           <xs:element name="clients" type="GlusterClients" 
minOccurs="0" maxOccurs="1" />
Line 3024:           <xs:element name="memory_info" 
type="GlusterBrickMemoryInfo" minOccurs="0" maxOccurs="1"/>
after rename of [1] in to [2] you can use ref="memory_info", using this change, 
non strongly typed clients (such as python) will be able to identify 
"memory_info" property type and codegen implementations won't have to carry 
static mappings such as "memory_info=GlusterBrickMemoryInfo"

[1] <xs:element name="gluster_brick_memory_info" type="GlusterBrickMemoryInfo"/>

[2] <xs:element name="memory_info" type="GlusterBrickMemoryInfo"/>
Line 3025:         </xs:sequence>
Line 3026:       </xs:extension>
Line 3027:     </xs:complexContent>
Line 3028:   </xs:complexType>


--
To view, visit http://gerrit.ovirt.org/11391
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie219c7cf59fec8a21a54f34959ee5966eed7d524
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Sahina Bose <[email protected]>
Gerrit-Reviewer: Kanagaraj M <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Sahina Bose <[email protected]>
Gerrit-Reviewer: Shireesh Anjal <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to