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

Reply via email to