Ori Liel has posted comments on this change.

Change subject: restapi: implement Quota REST-API
......................................................................


Patch Set 1:

(11 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 10: clusterquotalimits
Remove this line, only necessary for root collections.


https://gerrit.ovirt.org/#/c/39904/1/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/QuotaResource.java
File 
backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/QuotaResource.java:

Line 7: 
Line 8: @Produces({ApiMediaType.APPLICATION_XML, ApiMediaType.APPLICATION_JSON, 
ApiMediaType.APPLICATION_X_YAML})
Line 9: public interface QuotaResource extends UpdatableResource<Quota> {
Line 10: 
Line 11:     @Path("storagequotalimits")
If you do decide to change the names to quotastoragelimit and 
clusterstoragelimit (see my comment in api.xsd), then off course you should 
change here as well.
Line 12:     public StorageQuotaLimitsResource getStorageQuotaLimitsResource();
Line 13: 
Line 14:     @Path("clusterquotalimits")
Line 15:     public ClusterQuotaLimitsResource getClusterQuotaLimitsResource();


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 10: storagequotalimits
Remove this, necessary only for root collections (collections which are 
directly under ../api)


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 4083:       </xs:extension>
Line 4084:     </xs:complexContent>
Line 4085:   </xs:complexType>
Line 4086: 
Line 4087:   <xs:element name="storage_quota_limit" type="StorageQuotaLimit" />
Maybe QuotaLimitStorage, QuotaLimitCluster are clearer names? By having the 
name start with "Quota" it's immediately clear that this entity is related to 
the quota feature.
Line 4088: 
Line 4089:   <xs:complexType name="StorageQuotaLimit">
Line 4090:     <xs:complexContent>
Line 4091:       <xs:extension base="BaseResource">


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 5481: specified
left over from copy-paste? ;)


Line 5487: }
A recent patch by Juan adds 'async' url param implicitly to all delete and 
update signatures. So this line is unnecessary.


Line 5518: specified
Remove the word 'specified'


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}
A recent patch by Juan adds 'async' url param implicitly to all delete and 
update signatures. So this line is unnecessary.
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 5545: storage_domain
I believe these should by mandatory arguments?


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}
A recent patch by Juan adds 'async' url param implicitly to all delete and 
update signatures. So this line is unnecessary.
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 5569:         optionalArguments:
Line 5570:           cluster_domain.id: 'xs:string'
Line 5571:           limit: 'xs:long'
Line 5572:         description: add a cluster limit to a specified Quota
Line 5573
Shouldn't these arguments be mandatory?


-- 
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

Reply via email to