Juan Hernandez has posted comments on this change.

Change subject: restapi: introducing the instance types rest part
......................................................................


Patch Set 6: Code-Review+1

(7 comments)

Looks good, but there are some minor issues in the RSDL metadata that I would 
like you to fix before merging. See the comments inline.

http://gerrit.ovirt.org/#/c/27776/6/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 49:       parameterType: VM
Line 50:       signatures:
Line 51:       - mandatoryArguments: {}
Line 52:         optionalArguments:
Line 53:           vm.instance_type.id|name: 'xs:string'
I'm not sure if these quotes are going to be accepted, can you remove them?

  vm.instance_type.id|name: xs:string
Line 54:           vm.name: xs:string
Line 55:           vm.cluster.id|name: xs:string
Line 56:           vm.timezone: xs:string
Line 57:           vm.os.boot--COLLECTION: {boot.dev: 'xs:string'}


Line 126:       parameterType: VM
Line 127:       signatures:
Line 128:       - mandatoryArguments: {vm.name: 'xs:string', 
vm.template.id|name: 'xs:string', vm.cluster.id|name: 'xs:string'}
Line 129:         optionalArguments:
Line 130:           vm.instance_type.id|name: 'xs:string'
Same.
Line 131:           vm.quota.id: xs:string
Line 132:           vm.timezone: xs:string
Line 133:           vm.os.boot--COLLECTION: {boot.dev: 'xs:string'}
Line 134:           vm.custom_properties.custom_property--COLLECTION: 
{custom_property.name: 'xs:string', custom_property.value: 'xs:string'}


Line 4887:       signatures: []
Line 4888:     urlparams:
Line 4889:       max: {context: matrix, type: 'xs:int', value: 'max results', 
required: false}
Line 4890:     headers: {}
Line 4891: - name: /instancetypes/{instance_type:id}|rel=get
In the URL everything should go without underscores:

  /instancetypes/{instancetype:id}|rel=get
Line 4892:   description: get the details of the instance type on the system
Line 4893:   request:
Line 4894:     body:
Line 4895:       parameterType: null


Line 4895:       parameterType: null
Line 4896:       signatures: []
Line 4897:     urlparams: {}
Line 4898:     headers: {}
Line 4899: - name: /instancetypes/{instance_type:id}|rel=delete
Same, no underscores in the URL.
Line 4900:   description: delete the instance type from the system
Line 4901:   request:
Line 4902:     body:
Line 4903:       parameterType: null


Line 4915:       - mandatoryArguments: {instance_type.name: 'xs:string'}
Line 4916:         optionalArguments:
Line 4917:           instance_type.memory: xs:long
Line 4918:           instance_type.cpu.topology.cores: xs:int
Line 4919:           instance_type.high_availability.enabled: 'xs:boolean'
Remove the quotes from the type.
Line 4920:           instance_type.origin: xs:string
Line 4921:           instance_type.high_availability.priority: 'xs:int'
Line 4922:           instance_type.console.enabled: xs:boolean
Line 4923:           instance_type.description: xs:string


Line 4917:           instance_type.memory: xs:long
Line 4918:           instance_type.cpu.topology.cores: xs:int
Line 4919:           instance_type.high_availability.enabled: 'xs:boolean'
Line 4920:           instance_type.origin: xs:string
Line 4921:           instance_type.high_availability.priority: 'xs:int'
Remove the quotes from the type.
Line 4922:           instance_type.console.enabled: xs:boolean
Line 4923:           instance_type.description: xs:string
Line 4924:           instance_type.os.boot--COLLECTION: {boot.dev: 'xs:string'}
Line 4925:           instance_type.cpu.topology.sockets: xs:int


http://gerrit.ovirt.org/#/c/27776/6/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendInstanceTypesResource.java
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendInstanceTypesResource.java:

Line 42:             return 
mapCollection(getBackendCollection(SearchType.InstanceType));
Line 43:         }
Line 44:     }
Line 45: 
Line 46:     @Override
Indentation of the methods below looks wrong.
Line 47:               public Response add(InstanceType instanceType) {
Line 48:                   validateParameters(instanceType, "name");
Line 49: 
Line 50:                   VmStatic vmStatic = getMapper(InstanceType.class, 
VmStatic.class).map(instanceType, new VmStatic());


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I45f464339e30258860f100240f34d1aa51e533ee
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Ori Liel <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[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