Ori Liel has posted comments on this change. Change subject: restapi: Add API to support warning for attached Storage Domains ......................................................................
Patch Set 4: (7 comments) http://gerrit.ovirt.org/#/c/36849/4//COMMIT_MSG Commit Message: Line 24: <?xml version="1.0" encoding="UTF-8" standalone="yes"?> Line 25: <storage_domain id="d97c4fc5-bf5d-41a3-94c6-8fe821f98ff0"> Line 26: <is_attached>false</is_attached> Line 27: </storage_domain> Line 28: I think it's worth adding, in case someone will wonder in the future, that this could not have been implemented as another field returned by GET, because the check which you are performing requires the connection-host to be provided as parameter by the client (I don't like it, but that's the way it is). Line 29: Change-Id: I4b12e5ea2abea66cb950e3cdd64baaaff72a6919 Line 30: Bug-Url: https://bugzilla.redhat.com/1179246 http://gerrit.ovirt.org/#/c/36849/4/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 1528: headers: Line 1529: Content-Type: {value: application/xml|json, required: true} Line 1530: Correlation-Id: {value: 'any string', required: false} Line 1531: - name: /storagedomains/{storagedomain:id}/isattached|rel=isattached Line 1532: description: Querying if the Storage Domain is already attached to a Data Center by the is_attached boolean field Please make clear in this description that this field exists on the storage server. The way it's written one might think that this is an ovirt field, and then wonder why fetching it requires an action (as opposed to it being returned from GET) Line 1533: request: Line 1534: body: Line 1535: parameterType: StorageDomain Line 1536: signatures: Line 1531: - name: /storagedomains/{storagedomain:id}/isattached|rel=isattached Line 1532: description: Querying if the Storage Domain is already attached to a Data Center by the is_attached boolean field Line 1533: request: Line 1534: body: Line 1535: parameterType: StorageDomain The parameter type is 'Action', like in all actions. See any action in this file for example. Line 1536: signatures: Line 1537: - mandatoryArguments: {storagedomain.host.id|name: 'xs:string', storagedomain.id: 'xs:string', storagedomain.id: 'xs:string'} Line 1538: optionalArguments: {storagedomain.name: 'xs:string'} Line 1539: description: Line 1533: request: Line 1534: body: Line 1535: parameterType: StorageDomain Line 1536: signatures: Line 1537: - mandatoryArguments: {storagedomain.host.id|name: 'xs:string', storagedomain.id: 'xs:string', storagedomain.id: 'xs:string'} You don't need to pass a storage-domain inside action. You already know the storage-domain from the context: .../api/storagedomains/XXX/isattached. All you need to pass is the host: <action> <host id="yyy"/> </action> Line 1538: optionalArguments: {storagedomain.name: 'xs:string'} Line 1539: description: Line 1540: urlparams: Line 1541: async: {context: matrix, type: 'xs:boolean', value: true|false, required: false} Line 1534: body: Line 1535: parameterType: StorageDomain Line 1536: signatures: Line 1537: - mandatoryArguments: {storagedomain.host.id|name: 'xs:string', storagedomain.id: 'xs:string', storagedomain.id: 'xs:string'} Line 1538: optionalArguments: {storagedomain.name: 'xs:string'} Don't need storage-domain name either, remove this Line 1539: description: Line 1540: urlparams: Line 1541: async: {context: matrix, type: 'xs:boolean', value: true|false, required: false} Line 1542: headers: Line 1540: urlparams: Line 1541: async: {context: matrix, type: 'xs:boolean', value: true|false, required: false} Line 1542: headers: Line 1543: Content-Type: {value: application/xml|json, required: true} Line 1544: Expect: {value: 201-created, required: false} Why Expect 201? You're not creating something new. Take an example from another 'action' in this file. Line 1545: Correlation-Id: {value: 'any string', required: false} Line 1546: - name: /storagedomains/{storagedomain:id}|rel=update Line 1547: description: update the storage domain Line 1548: request: http://gerrit.ovirt.org/#/c/36849/4/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageDomainsResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageDomainsResource.java: Line 52: extends AbstractBackendCollectionResource<StorageDomain, org.ovirt.engine.core.common.businessentities.StorageDomain> Line 53: implements StorageDomainsResource { Line 54: Line 55: static final String[] SUB_COLLECTIONS = { "permissions", "files", "templates", "vms", "disks", Line 56: "storageconnections", "images", "disksnapshots", "diskprofiles", "isattached" }; only sub-collections go here, not actions. Line 57: Line 58: private StorageDomain storageDomain = null; // utility variable; used in the context of a single activation of Line 59: // remove() Line 60: -- To view, visit http://gerrit.ovirt.org/36849 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4b12e5ea2abea66cb950e3cdd64baaaff72a6919 Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Ori Liel <ol...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
