Daniel Erez has posted comments on this change. Change subject: restapi: OpenStack image provider ......................................................................
Patch Set 6: (5 comments) http://gerrit.ovirt.org/#/c/33975/6//COMMIT_MSG Commit Message: Line 64: GET /openstackimageproviders/{provider:id}/certificates Line 65: <certificates> Line 66: <certificate id="{host:id}">...</certificate> Line 67: ... Line 68: </fcertificates> s/fcertificates/certificates Line 69: Line 70: GET /openstackimageproviders/{provider:id}/certificates/{certificate:id} Line 71: <certificate id="{certificate:id}"> Line 72: <subject>CN=glance.example.com</subject> http://gerrit.ovirt.org/#/c/33975/6/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/openstack/BackendOpenStackImageProviderHelper.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/openstack/BackendOpenStackImageProviderHelper.java: Line 37: */ Line 38: public static Guid getStorageDomainId(BackendResource resource, String providerId) { Line 39: // The backend doesn't have any mechanism to obtain the images other than listing the images provided by the Line 40: // storage domain that is created for the provider, and the only way to find that provider is to iterate the Line 41: // complete list. This is potentially very slow, so it should be improved in the future. I think you can use 'StorageDomainDAO -> getAllByConnectionId' (confusing naming.. but this stored procedure compares against 'storage' column). It would require a new simple query, so probably can be postponed to a later patch. Any way, can't we use the storage domain id specified by the user (at least in some cases)? Line 42: Guid storageDomainId = null; Line 43: List<StorageDomain> storageDomains = Line 44: resource.runQuery(VdcQueryType.GetAllStorageDomains, new VdcQueryParametersBase()).getReturnValue(); Line 45: for (StorageDomain storageDomain : storageDomains) { http://gerrit.ovirt.org/#/c/33975/6/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/openstack/BackendOpenStackImageProviderResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/openstack/BackendOpenStackImageProviderResource.java: Line 71: } Line 72: Line 73: @Override Line 74: public Response testConnectivity(Action action) { Line 75: Provider provider = BackendExternalProviderHelper.getProvider(this, id); Can we extract this code to an upper level? It should be generic for all providers.. Line 76: ProviderParameters parameters = new ProviderParameters(provider); Line 77: return performAction(VdcActionType.TestProviderConnectivity, parameters); Line 78: } Line 79: Line 78: } Line 79: Line 80: @Override Line 81: public Response importCertificates(Action action) { Line 82: Provider provider = BackendExternalProviderHelper.getProvider(this, id); same Line 83: ProviderParameters parameters = new ProviderParameters(provider); Line 84: return performAction(VdcActionType.ImportProviderCertificateChain, parameters); Line 85: } Line 86: http://gerrit.ovirt.org/#/c/33975/6/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/openstack/BackendOpenStackImageResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/openstack/BackendOpenStackImageResource.java: Line 61: Line 62: @Override Line 63: public Response doImport(Action action) { Line 64: validateParameters(action, "storageDomain.id|name"); Line 65: Guid storageDomainId = BackendOpenStackImageProviderHelper.getStorageDomainId(this, providerId); can't we use the specified storage domain id instead of fetching it? Line 66: ImportRepoImageParameters parameters = new ImportRepoImageParameters(); Line 67: parameters.setSourceRepoImageId(id); Line 68: parameters.setSourceStorageDomainId(storageDomainId); Line 69: parameters.setStoragePoolId(getDataCenterId(getStorageDomainId(action))); -- To view, visit http://gerrit.ovirt.org/33975 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I77fca2208b18d54f9177ec8b0178768a6b815f20 Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Juan Hernandez <juan.hernan...@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