Juan Hernandez has posted comments on this change.

Change subject: jsonrpc: Stomp changes in vdsbroker
......................................................................


Patch Set 7:

(3 comments)

For the RESTAPI the protocol shouldn't be an integer, but an string, 
implementing using the mechanism that the RESTAPI uses for enums. Look at how 
DiskFormat is implemented, for example.

http://gerrit.ovirt.org/#/c/26783/7/backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd
File 
backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd:

Line 1550:           <xs:element name="memory" type="xs:long" minOccurs="0"/>
Line 1551:           <xs:element name="max_scheduling_memory" type="xs:long" 
minOccurs="0"/>
Line 1552:           <xs:element name="summary" type="VmSummary" minOccurs="0" 
maxOccurs="1"/>
Line 1553:           <xs:element name="override_iptables" type="xs:boolean" 
minOccurs="0" maxOccurs="1"/>
Line 1554:           <xs:element name="protocol" type="xs:unsignedShort" 
minOccurs="0" maxOccurs="1"/>
Don't use an integer here, use xs:string instead and the mechanism that the 
RESTAPI uses for enums. Look at DiskFormat, for example.
Line 1555:           <!-- when installing a host, optionally reboot the host -->
Line 1556:           <xs:element name="reboot_after_installation" 
type="xs:boolean" minOccurs="0"/>
Line 1557:           <xs:element name="os" type="OperatingSystem" minOccurs="0" 
maxOccurs="1"/>
Line 1558:           <xs:element ref="hooks" minOccurs="0"/>


http://gerrit.ovirt.org/#/c/26783/7/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 2564:           host.power_management.enabled: 'xs:boolean', 
host.power_management.address: 'xs:string', host.power_management.username: 
'xs:string', host.power_management.automatic_pm_enabled: 'xs:boolean',
Line 2565:           host.power_management.password: 'xs:string', 
host.power_management.options.option--COLLECTION: {option.name: 'xs:string', 
option.value: 'xs:string'},
Line 2566:           host.power_management.pm_proxy--COLLECTION: {propietary : 
'xs:string'}, host.power_management.agents.agent--COLLECTION:{type: 'xs:string',
Line 2567:           address: 'xs:string', username: 'xs:string', password: 
'xs:string', options.option--COLLECTION: {option.name: 'xs:string', 
option.value: 'xs:string'}}, host.reboot_after_installation: 'xs:boolean', 
host.override_iptables: 'xs:boolean',
Line 2568:           host.power_management.kdump_detection: 'xs:boolean', 
host.protocol: 'xs:int'}
This should be xs:string.
Line 2569:         description: add a new host to the system providing the host 
root password. This has been deprecated and provided for backwards compatibility
Line 2570:       - mandatoryArguments: {host.name: 'xs:string', host.address: 
'xs:string', host.cluster.id|name: 'xs:string'}
Line 2571:         optionalArguments: {host.comment: 'xs:string', 
host.ssh.port: 'xs:int', host.ssh.fingerprint: 'xs:string', 
host.ssh.authentication_method: 'xs:string',
Line 2572:           host.ssh.user.user_name: 'xs:string', 
host.ssh.user.password: 'xs:string', host.port: 'xs:int',


Line 2574:           host.power_management.enabled: 'xs:boolean', 
host.power_management.address: 'xs:string', host.power_management.username: 
'xs:string',
Line 2575:           host.power_management.password: 'xs:string', 
host.power_management.options.option--COLLECTION: {option.name: 'xs:string', 
option.value: 'xs:string'},
Line 2576:           host.power_management.pm_proxy--COLLECTION: {propietary : 
'xs:string'}, host.power_management.agents.agent--COLLECTION:{type: 
'xs:string', address: 'xs:string',
Line 2577:           username: 'xs:string', password: 'xs:string', 
options.option--COLLECTION: {option.name: 'xs:string', option.value: 
'xs:string'}}, host.reboot_after_installation: 'xs:boolean', 
host.override_iptables: 'xs:boolean',
Line 2578:           host.power_management.kdump_detection: 'xs:boolean', 
host.protocol: 'xs:int'}
This should be xs:string.
Line 2579:         description: add a new host to the system providing the ssh 
password or fingerprint
Line 2580:     urlparams: {}
Line 2581:     headers:
Line 2582:       Content-Type: {value: application/xml|json, required: true}


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If78de6620ba6891543531ac8ddd633b67828a89c
Gerrit-PatchSet: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: Barak Azulay <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[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