Ori Liel has posted comments on this change. Change subject: restapi: making gluster enums lower case in request/response ......................................................................
Patch Set 1: Currently (before this change) Gluster enums are not handled like other enums in the API. I believe this should change, and Gluster enums should be handled like the other enums in the API. This is the way enums are handled in the API: For every Backend enum (e.g: org.ovirt.engine.core.common.businessentities.gluster.GlusterVolumeType), there's a corresponding rest-api entity (e.g: org.ovirt.engine.api.model.GlusterVolumeType). This is because Backend enums are considered internal; they are written with less API awareness, so sometimes their values aren't informative enough, and also, they are allowed to change, and this should not break the API. Gluster enums also exist separately in Backend and in REST, so far so good. The thing is, we can not count on the strings to be the same in both enums; for example, the following situation is possible: (Backend) org.ovirt.engine.core.common.businessentities.gluster.GlusterVolumeType.STRIPE <--> (REST) org.ovirt.engine.api.model.GlusterVolumeType.STRIPED (notice 'D' at the end, meaning, the strings are different). Since the strings will not always be equal, we can't simply take the string from enum A (e.g: "STRIPE") and look for it in enum B, and consider that valid mapping. What you'll see with other enums in the API, is that for each one there's a pair of mapping functions in the relevant Mapper class, which explicitly maps the values from the Backend enum to the REST enum and vica-versa. (e.g: "if value==STRIPE return GlusterVolumeType.STRIPE"...) In cases where the value is read only, such as status (the user can't set status, only see it), there will be only one mapping function for the direction Backend-->REST. This seems not to be the case with any of the Gluster enums. The mappings seem to all rely on string matching, which is inconsistent with the rest of the API and will break if ever the values in the Backend enums change. Can you please make this enum refactoring patch move Gluster enums towards the same behavior as in the rest of the API? You can take example from StorageType enum in StorageDomainMapper, or many other enums in rest-api. Thanks, Ori. -- 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: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: 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> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
