Shireesh Anjal has posted comments on this change. Change subject: restapi: Added Gluster entities in REST schema ......................................................................
Patch Set 7: (5 inline comments) Need clarification on few of the comments. .................................................... File backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd Line 2394: <xs:element name="volume_type" type="GlusterVolumeType" minOccurs="1" maxOccurs="1"/> I can see a few enumerations defined in the schema e.g. StatisticType, StatisticUnit, ValueType, HttpMethod, though none of these get listed in the capabilities api. I also see that such enum definitions in schema generate corresponding Java Enum types in /restapi-definition/src/main/java/org/ovirt/engine/api/model If above understanding is correct, I think it will be useful to define the enum in schema, and just introduce appropriate changes in BackendCapabilitiesResource I think the clients can still send a "String" in the corresponding field, which should get converted to the enum automatically by the jaxb annotations on the enum. Only reason I can think of to avoid using an enum is if the system cannot gracefully return an error response in case clients send a value that is not supported by the enum, and hence the xml de-serialization fails. Do let me know if I'm missing something here. Line 2399: <xs:element name="access_protocols" type="xs:string" minOccurs="0" maxOccurs="1"/> Will do. Line 2400: <xs:element name="access_control_list" type="xs:string" minOccurs="0" maxOccurs="1"/> Will do. Line 2476: <xs:element ref="gluster_brick" minOccurs="1" maxOccurs="unbounded"/> At the lowest level (GlusterFS), it is simply not possible that there exists a volume without a single brick. So I don't see why one should not add the corresponding constraint in the schema. Based on this comment, and the previous one about enumerations, I'm getting a feeling that the xml schema should be used only to define the structure of entities, but not to define any constraints that can result in failure of de-serialization if data is incorrect, probably because we currently don't have a mechanism to generate meaningful errors / responses out of such failures. If it is so, I will change the gluster schema entities accordingly. Line 2509: <xs:element ref="gluster_option" minOccurs="1" maxOccurs="unbounded"/> "gluster_options" has a minOccurs of "0". So what I'm trying to say here is that, if you pass the "gluster_options" tag in data, then it must have at least one "gluster_option" inside it. Otherwise there is no point in sending the parent tag itself. Having said that, now I think that it could be easier for clients to consume the api if we make minOccurs="0" in this case, and it won't cause any problems in the business logic either. So I'll change this one straight away. -- To view, visit http://gerrit.ovirt.org/3360 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I93c4df85068c3ed1ebfa35124bdfc2ae8f4482c8 Gerrit-PatchSet: 7 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Shireesh Anjal <san...@redhat.com> Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Livnat Peer <lp...@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: Yair Zaslavsky <yzasl...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
