Juan Hernandez has posted comments on this change. Change subject: restapi: rest support for MacPool ......................................................................
Patch Set 2: (11 comments) http://gerrit.ovirt.org/#/c/28706/2//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2014-06-13 13:02:59 +0200 Line 4: Commit: Martin Mucha <[email protected]> Line 5: CommitDate: 2014-06-13 15:42:28 +0200 Line 6: Line 7: restapi: rest support for MacPool There are changes not related to the RESTAPI in this patch. Either split the patch into several patches or change the subject, something like this: core, restapi: Support for MacPool Also, please explain a bit the purpose of the patch in the commit message. Line 8: Line 9: Change-Id: Id343433198dadd209933b69012303c51acb4c1e1 Line 10: Bug-Url: https://bugzilla.redhat.com/1078844 http://gerrit.ovirt.org/#/c/28706/2/backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd File backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd: Line 1228: <!-- Mac Pools--> Line 1229: <xs:element name="mac_pool" type="MacPool"/> Line 1230: <xs:element name="mac_pools" type="MacPools"/> Line 1231: Line 1232: <xs:complexType name="MacPool"> Please indent using two spaces per level. Line 1233: <xs:complexContent> Line 1234: <xs:extension base="BaseResource"> Line 1235: <xs:sequence> Line 1236: <xs:element name="shared" type="xs:boolean"/> Line 1233: <xs:complexContent> Line 1234: <xs:extension base="BaseResource"> Line 1235: <xs:sequence> Line 1236: <xs:element name="shared" type="xs:boolean"/> Line 1237: <xs:element name="allow_duplicates" type="xs:boolean" minOccurs="1"/> Please be explicit with "maxOccurs" and "minOccurs" in all elements. Line 1238: <xs:element name="default_pool" type="xs:boolean" minOccurs="0"/> Line 1239: <xs:element name="ranges"> Line 1240: <xs:complexType> Line 1241: <xs:sequence> Line 1234: <xs:extension base="BaseResource"> Line 1235: <xs:sequence> Line 1236: <xs:element name="shared" type="xs:boolean"/> Line 1237: <xs:element name="allow_duplicates" type="xs:boolean" minOccurs="1"/> Line 1238: <xs:element name="default_pool" type="xs:boolean" minOccurs="0"/> What is the meaning of this element? Can't it just be "default"? Line 1239: <xs:element name="ranges"> Line 1240: <xs:complexType> Line 1241: <xs:sequence> Line 1242: <xs:element name="range" maxOccurs="unbounded"> Line 1236: <xs:element name="shared" type="xs:boolean"/> Line 1237: <xs:element name="allow_duplicates" type="xs:boolean" minOccurs="1"/> Line 1238: <xs:element name="default_pool" type="xs:boolean" minOccurs="0"/> Line 1239: <xs:element name="ranges"> Line 1240: <xs:complexType> The SDK generators will probably choke on this. Please define it as a complex type: <xs:complexType name="MacAddressRange"> ... </xs:complexType> <xs:complexType name="MacAddressRanges"> ... </xs:complexType> <xs:element name="range" type="MacAddressRange"/> <xs:element name="ranges" type="MacAddressRanges"/> Then use references to these elements: <xs:element ref="ranges"/> To avoid potential conflicts with other types of "ranges" introduced in the future you may want to name them "mac_range" and "mac_ranges", or "mac_address_range" and "mac_address_ranges". Line 1241: <xs:sequence> Line 1242: <xs:element name="range" maxOccurs="unbounded"> Line 1243: <xs:complexType> Line 1244: <xs:sequence> Line 1273: Line 1274: <xs:simpleType name="MacAddress"> Line 1275: <xs:restriction base="xs:string"> Line 1276: <xs:pattern value="[0-9a-fA-F]{2}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}"/> Line 1277: </xs:restriction> We don't use restrictions or patterns in the XML schema of the RESTAPI, as the SDKs won't understand them. Use a plain string, and a comment explaining what is the expected format. Line 1278: </xs:simpleType> Line 1279: Line 1280: Line 1281: <!-- Data Centers --> Line 1294: <xs:element name="storage_format" type="xs:string" minOccurs="0" /> Line 1295: <xs:element name="version" type="Version" minOccurs="0" /> Line 1296: <xs:element name="supported_versions" type="SupportedVersions" minOccurs="0" /> Line 1297: <xs:element ref="status" minOccurs="0" maxOccurs="1" /> Line 1298: <xs:element name="mac_pool_id" type="xs:string" minOccurs="0"/> This should be a link to the mac pool: <xs:element ref="mac_pool"/> It should be then displayed as follows: <data_center id="..." href="..."> <name>mydc</name> <mac_pool id="1234" href="/macpools/1234"/> ... </data_center> And when modifying or creating a data center the caller should send it as follows: <data_center> <name>mydc</name> <mac_pool id="1234"/> </data_center> Or using the name: <data_center> <name>mydc</name> <mac_pool> <name>mypool</name> </mac_pool> </data_center> Line 1299: <!-- also rel="files" and rel="storagedomains" links --> Line 1300: </xs:sequence> Line 1301: </xs:extension> Line 1302: </xs:complexContent> http://gerrit.ovirt.org/#/c/28706/2/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 1912: description: delete the specified data center from the system Line 1913: urlparams: {} Line 1914: headers: Line 1915: Content-Type: {value: application/xml|json, required: true} Line 1916: - name: /macpools/{datacenter:id}|rel=update Should be: /macpools/{macpool:id}|rel=update Line 1917: description: update the specified mac pool in the system Line 1918: request: Line 1919: body: Line 1920: parameterType: MacPool Line 1935: description: update the specified mac pool in the system Line 1936: urlparams: {} Line 1937: headers: Line 1938: Content-Type: {value: application/xml|json, required: true} Line 1939: - name: /macpools/{datacenter:id}|rel=add Should be: /macpools/{macpool:id}|rel=add Line 1940: description: add the specified mac pool into the system Line 1941: request: Line 1942: body: Line 1943: parameterType: MacPool Line 1947: macpool.ranges: Line 1948: - range: Line 1949: from: xs:string Line 1950: to: xs:string Line 1951: comment: xs:string The generators of the SDKs won't understand this. Please use the --COLLECTION syntax: macpool.range--COLLECTION: {range.from: 'xs:string', range.to: 'xs:string', range.comment: 'xs:string'} Line 1952: optionalArguments: Line 1953: macpool.description: xs:string Line 1954: macpool.comment: xs:string Line 1955: macpool.shared: xs:boolean Line 2026: - mandatoryArguments: Line 2027: datacenter.name: 'xs:string' Line 2028: datacenter.version.major: 'xs:int' Line 2029: datacenter.version.minor: 'xs:int' Line 2030: datacenter.mac_pool_id: xs:string This should be a link to the MAC pool: datacenter.macpool.id|name: xs:string Line 2031: optionalArguments: Line 2032: datacenter.storage_type--DEPRECATED: xs:string Line 2033: datacenter.local: xs:boolean Line 2034: datacenter.description: xs:string -- To view, visit http://gerrit.ovirt.org/28706 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id343433198dadd209933b69012303c51acb4c1e1 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <[email protected]> Gerrit-Reviewer: Juan Hernandez <[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
