Ori Liel has posted comments on this change.

Change subject: restapi: making gluster enums lower case in request/response
......................................................................


Patch Set 5: (1 inline comment)

....................................................
File 
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/GlusterBrickMapper.java
Line 69:             return GlusterState.DOWN.name();
Line 70:         default:
Line 71:             return null;
Line 72:         }
Line 73:     }
It's a step in the right direction, but it would be best to be uniform with the 
rest of the application. 

For full uniformity: 

1) Maintain GlusterStatus.java enum in the API, and map Backend values to it. 
2) Instead of status string, use the 'Status' object (I didn't mention this 
before)

The easiest way is to follow an existing example. For example, see this from  
VmMapper: 

        model.setStatus(StatusUtils.create(map(entity.getstatus(), null)));

1) Backend VM status is mapped to a rest-api status (an enum)
2) The enum is sent to StatusUtils.create() which creates a 'Status' object 
3) The 'Status' object is set in the rest-api entity (and later the entity is 
returned)

Note that this woule require changing in api.xsd, for GlusterBrick: 

  <xs:element name="state" type="xs:string" minOccurs="0" maxOccurs="1"/>

to: 

  <xs:element name="state" type="Status" minOccurs="0" maxOccurs="1"/>

And for uniformity, best also make this change to GlusterVolume state.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1aeec51d82ad650b9e1364295e6794e0b0861618
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Kanagaraj M <kmayi...@redhat.com>
Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com>
Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Ori Liel <ol...@redhat.com>
Gerrit-Reviewer: Shireesh Anjal <san...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to