Maor Lipchuk has posted comments on this change.

Change subject: core: OpenStackVolumeProviderProxy and CINDERStorageHelper
......................................................................


Patch Set 6:

(5 comments)

https://gerrit.ovirt.org/#/c/38911/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/storage/OpenStackVolumeProviderProxy.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/storage/OpenStackVolumeProviderProxy.java:

Line 55:     }
Line 56: 
Line 57:     @Override
Line 58:     public void onRemoval() {
Line 59:         List<StorageDomain> storageDomains = 
getDbFacade().getStorageDomainDao().getAllByConnectionId(provider.getId());
Please add an empty line before a comment
Line 60:         // removing the static and dynamic storage domain entries
Line 61:         for (StorageDomain storageDomainEntry : storageDomains) {
Line 62:             
getDbFacade().getStorageDomainDao().remove(storageDomainEntry.getId());
Line 63:         }


Line 56: 
Line 57:     @Override
Line 58:     public void onRemoval() {
Line 59:         List<StorageDomain> storageDomains = 
getDbFacade().getStorageDomainDao().getAllByConnectionId(provider.getId());
Line 60:         // removing the static and dynamic storage domain entries
/s/remove/Remove
Line 61:         for (StorageDomain storageDomainEntry : storageDomains) {
Line 62:             
getDbFacade().getStorageDomainDao().remove(storageDomainEntry.getId());
Line 63:         }
Line 64:     }


https://gerrit.ovirt.org/#/c/38911/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/CINDERStorageHelper.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/CINDERStorageHelper.java:

Line 74:         });
Line 75:     }
Line 76: 
Line 77:     public void activateCinderDomain(Guid storageDomainId, Guid 
storagePoolId) {
Line 78:         OpenStackVolumeProviderProxy proxy = 
OpenStackVolumeProviderProxy.getFromStorageDomainId(storageDomainId);
How you can be sure that the proxy is not null?
Line 79:         try {
Line 80:             proxy.testConnection();
Line 81:             updateCinderDomainStatus(storageDomainId, storagePoolId, 
StorageDomainStatus.Active);
Line 82:         } catch (VdcBLLException e) {


Line 80:             proxy.testConnection();
Line 81:             updateCinderDomainStatus(storageDomainId, storagePoolId, 
StorageDomainStatus.Active);
Line 82:         } catch (VdcBLLException e) {
Line 83:             AuditLogableBase loggable = new AuditLogableBase();
Line 84:             loggable.addCustomValue("CinderException", 
e.getCause().getCause() != null ?
Do we have to call getCause().getCause()? why getCause() is not enough?
Line 85:                     e.getCause().getCause().getMessage() : 
e.getCause().getMessage());
Line 86:             new AuditLogDirector().log(loggable, 
AuditLogType.CINDER_PROVIDER_ERROR);
Line 87:             throw e;
Line 88:         }


Line 103:         });
Line 104:     }
Line 105: 
Line 106:     private StoragePoolIsoMapDAO getStoragePoolIsoMapDAO() {
Line 107:         return DbFacade.getInstance().getStoragePoolIsoMapDao();
use getDbFacade
Line 108:     }
Line 109: 
Line 110:     private static DbFacade getDbFacade() {
Line 111:         return DbFacade.getInstance();


-- 
To view, visit https://gerrit.ovirt.org/38911
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie94f2f8581174cf820d908ba3b51de7564d8a0d4
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[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