Juan Hernandez has posted comments on this change. Change subject: restapi: implement Quota REST-API ......................................................................
Patch Set 1: (20 comments) https://gerrit.ovirt.org/#/c/39904/1/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/ClusterQuotaLimitsResource.java File backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/ClusterQuotaLimitsResource.java: Line 6: Line 7: import org.ovirt.engine.api.model.ClusterQuotaLimit; Line 8: import org.ovirt.engine.api.model.ClusterQuotaLimits; Line 9: Line 10: @Path("/clusterquotalimits") This isn't a top level collection, this annotation isn't needed. Line 11: @Produces({ ApiMediaType.APPLICATION_XML, ApiMediaType.APPLICATION_JSON, ApiMediaType.APPLICATION_X_YAML }) Line 12: public interface ClusterQuotaLimitsResource extends QuotaLimitsResource<ClusterQuotaLimits, ClusterQuotaLimit> { Line 13: @Override Line 14: @Path("{id}") https://gerrit.ovirt.org/#/c/39904/1/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/StorageQuotaLimitsResource.java File backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/StorageQuotaLimitsResource.java: Line 6: Line 7: import org.ovirt.engine.api.model.StorageQuotaLimit; Line 8: import org.ovirt.engine.api.model.StorageQuotaLimits; Line 9: Line 10: @Path("/storagequotalimits") This isn't a top level collection, so this annotation isn't needed. Line 11: @Produces({ ApiMediaType.APPLICATION_XML, ApiMediaType.APPLICATION_JSON, ApiMediaType.APPLICATION_X_YAML }) Line 12: public interface StorageQuotaLimitsResource extends QuotaLimitsResource<StorageQuotaLimits, StorageQuotaLimit> { Line 13: @Override Line 14: @Path("{id}") https://gerrit.ovirt.org/#/c/39904/1/backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd File backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd: Line 4060: <xs:element ref="users" minOccurs="0" maxOccurs="1"/> Line 4061: <xs:element name="cluster_soft_limit_pct" type="xs:int" minOccurs="0" maxOccurs="1"/> Line 4062: <xs:element name="cluster_hard_limit_pct" type="xs:int" minOccurs="0" maxOccurs="1"/> Line 4063: <xs:element name="storage_soft_limit_pct" type="xs:int" minOccurs="0" maxOccurs="1"/> Line 4064: <xs:element name="storage_hard_limit_pct" type="xs:int" minOccurs="0" maxOccurs="1"/> Is "xs:int" a good type for a percentage in this case? In a quota of 1 TiB, for example, 1% is approx 10 GiB, maybe too big a rounding error. Line 4065: </xs:sequence> Line 4066: </xs:extension> Line 4067: </xs:complexContent> Line 4068: </xs:complexType> https://gerrit.ovirt.org/#/c/39904/1/backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml File backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml: Line 5465: body: Line 5466: parameterType: null Line 5467: signatures: [] Line 5468: - name: /datacenters/{datacenter:id}/quotas|rel=get Line 5469: description: get a list of Quotas for specified Data Center Remove from here ... Line 5470: request: Line 5471: body: Line 5472: parameterType: null Line 5473: signatures: [] Line 5469: description: get a list of Quotas for specified Data Center Line 5470: request: Line 5471: body: Line 5472: parameterType: null Line 5473: signatures: [] ... to here. These empty lists aren't needed now. Line 5474: - name: /datacenters/{datacenter:id}/quotas/{quota:id}|rel=get Line 5475: description: get the details of the specified Quota Line 5476: request: Line 5477: body: Line 5475: description: get the details of the specified Quota Line 5476: request: Line 5477: body: Line 5478: parameterType: null Line 5479: signatures: [] Same. Line 5480: - name: /datacenters/{datacenter:id}/quotas/{quota:id}|rel=delete Line 5481: description: delete the specified user defined scheduling policy in the system Line 5482: request: Line 5483: body: Line 5477: body: Line 5478: parameterType: null Line 5479: signatures: [] Line 5480: - name: /datacenters/{datacenter:id}/quotas/{quota:id}|rel=delete Line 5481: description: delete the specified user defined scheduling policy in the system Remove from here ... Line 5482: request: Line 5483: body: Line 5484: parameterType: null Line 5485: signatures: [] Line 5483: body: Line 5484: parameterType: null Line 5485: signatures: [] Line 5486: urlparams: Line 5487: async: {context: matrix, type: 'xs:boolean', value: true|false, required: false} ... to here. Emty lists aren't needed, and the "async" parameter is added implicitly, so this isn't needed. Line 5488: - name: /datacenters/{datacenter:id}/quotas|rel=add Line 5489: description: add a new Quota to a Data Center Line 5490: request: Line 5491: body: Line 5490: request: Line 5491: body: Line 5492: parameterType: Quota Line 5493: signatures: Line 5494: - mandatoryArguments: {quota.name: 'xs:string'} Use indentation instead of curly brackets: - mandatoryArguments: quota.name: xs:string Line 5495: optionalArguments: Line 5496: quota.description: xs:string Line 5497: quota.cluster_soft_limit_pct: xs:int Line 5498: quota.cluster_hard_limit_pct: xs:int Line 5518: description: get a list of specified storage limits of a Quota Line 5519: request: Line 5520: body: Line 5521: parameterType: null Line 5522: signatures: [] Remove empty things. Line 5523: - name: /datacenters/{datacenter:id}/quotas/{quota:id}/storagequotalimits/{storagequotalimit:id}|rel=get Line 5524: description: get a specified storage limit of a Quota Line 5525: request: Line 5526: body: Line 5524: description: get a specified storage limit of a Quota Line 5525: request: Line 5526: body: Line 5527: parameterType: null Line 5528: signatures: [] Remove empty things. Line 5529: - name: /datacenters/{datacenter:id}/quotas/{quota:id}/storagequotalimits/{storagequotalimit:id}|rel=delete Line 5530: description: delete a storage limit from a Quota Line 5531: request: Line 5532: body: Line 5532: body: Line 5533: parameterType: null Line 5534: signatures: [] Line 5535: urlparams: Line 5536: async: {context: matrix, type: 'xs:boolean', value: true|false, required: false} Remove empty things and "async". Line 5537: - name: /datacenters/{datacenter:id}/quotas/{quota:id}/storagequotalimits|rel=add Line 5538: description: add a storage limit to a specified Quota Line 5539: request: Line 5540: body: Line 5542: signatures: Line 5543: - mandatoryArguments: {} Line 5544: optionalArguments: Line 5545: storage_domain.id: 'xs:string' Line 5546: limit: 'xs:long' Don't use quotes around "xs:...". Line 5547: description: add a storage limit to a specified Quota Line 5548: - name: /datacenters/{datacenter:id}/quotas/{quota:id}/clusterquotalimits/{clusterquotalimit:id}|rel=get Line 5549: description: get a specified cluster limit of a Quota Line 5550: request: Line 5549: description: get a specified cluster limit of a Quota Line 5550: request: Line 5551: body: Line 5552: parameterType: null Line 5553: signatures: [] Remove empty things. Line 5554: - name: /datacenters/{datacenter:id}/quotas/{quota:id}/clusterquotalimits/{clusterquotalimit:id}|rel=delete Line 5555: description: delete a cluster limit from a Quota Line 5556: request: Line 5557: body: Line 5557: body: Line 5558: parameterType: null Line 5559: signatures: [] Line 5560: urlparams: Line 5561: async: {context: matrix, type: 'xs:boolean', value: true|false, required: false} Remove empty things and "async". Line 5562: - name: /datacenters/{datacenter:id}/quotas/{quota:id}/clusterquotalimits|rel=add Line 5563: description: add a cluster limit to a specified Quota Line 5564: request: Line 5565: body: Line 5567: signatures: Line 5568: - mandatoryArguments: {} Line 5569: optionalArguments: Line 5570: cluster_domain.id: 'xs:string' Line 5571: limit: 'xs:long' Don't use quotes around "xs:...". https://gerrit.ovirt.org/#/c/39904/1/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/QuotaMapper.java File backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/QuotaMapper.java: Line 74: StorageQuotaLimit template) { Line 75: if (template == null) { Line 76: assert (false) : "StorageQuotaLimit cannot be null"; Line 77: return null; Line 78: } This, like all mappers, should create the model if no template is given: StorageQuotaLimit model = template != null? template: new StorageQuotaLimit(); Same in the other mappers inside this class. Line 79: Guid guid = GuidUtils.asGuid(template.getId()); Line 80: // global Line 81: if (guid.equals(entity.getId())) { Line 82: map(template, entity.getGlobalQuotaStorage(), null, entity.getStoragePoolId().toString(), entity.getId() Line 75: if (template == null) { Line 76: assert (false) : "StorageQuotaLimit cannot be null"; Line 77: return null; Line 78: } Line 79: Guid guid = GuidUtils.asGuid(template.getId()); Don't assume that the incoming "template" object has an id, it may come from the client and the client may not send it. Check before using it. Line 80: // global Line 81: if (guid.equals(entity.getId())) { Line 82: map(template, entity.getGlobalQuotaStorage(), null, entity.getStoragePoolId().toString(), entity.getId() Line 83: .toString()); Line 93: } Line 94: return template; Line 95: } Line 96: Line 97: private static void map(StorageQuotaLimit template, The name for this ^ parameter should probably be "model", not "template" as it isn't used as a template in this context. Line 98: QuotaStorage quotaStorage, Line 99: String storageDomainId, Line 100: String dataCenterId, Line 101: String quotaId) { Line 140: if (template == null) { Line 141: assert (false) : "ClusterQuotaLimit cannot be null"; Line 142: return null; Line 143: } Line 144: Guid guid = GuidUtils.asGuid(template.getId()); Create the model object and check the id, see the previous comments. Line 145: // global Line 146: if (guid.equals(entity.getId())) { Line 147: map(template, entity.getGlobalQuotaVdsGroup(), null, entity.getStoragePoolId().toString(), entity.getId() Line 148: .toString()); -- To view, visit https://gerrit.ovirt.org/39904 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifaead35839c2c5f89f1551e67e58a58aa253cab5 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Gilad Chaplik <[email protected]> Gerrit-Reviewer: Gilad Chaplik <[email protected]> Gerrit-Reviewer: Juan Hernandez <[email protected]> Gerrit-Reviewer: Ori Liel <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
