Shubhendu Tripathi has posted comments on this change.

Change subject: engine : bll command to set geo-rep config
......................................................................


Patch Set 5:

(4 comments)

http://gerrit.ovirt.org/#/c/35999/5/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GetGlusterVolumeGeoRepConfigListQuery.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GetGlusterVolumeGeoRepConfigListQuery.java:

Line 19:     }
Line 20: 
Line 21:     @Override
Line 22:     protected void executeQueryCommand() {
Line 23:         GlusterGeoRepDao geoRepDb = getGeoRepDao();
%s/geoRepDb/geoRepDao/g
Line 24:         Guid sessionId = getParameters().getId();
Line 25:         GlusterGeoRepSession session = geoRepDb.getById(sessionId);
Line 26:         GlusterVolumeEntity masterVolume = 
getGlusterVolumeDao().getById(session.getMasterVolumeId());
Line 27:         VDSReturnValue returnValue = 
runVdsCommand(VDSCommandType.GetGlusterVolumeGeoRepConfigList, new 
GlusterVolumeGeoRepSessionVDSParameters(getUpServerId(masterVolume.getClusterId()),
 sessionId, masterVolume.getName()));


Line 23:         GlusterGeoRepDao geoRepDb = getGeoRepDao();
Line 24:         Guid sessionId = getParameters().getId();
Line 25:         GlusterGeoRepSession session = geoRepDb.getById(sessionId);
Line 26:         GlusterVolumeEntity masterVolume = 
getGlusterVolumeDao().getById(session.getMasterVolumeId());
Line 27:         VDSReturnValue returnValue = 
runVdsCommand(VDSCommandType.GetGlusterVolumeGeoRepConfigList, new 
GlusterVolumeGeoRepSessionVDSParameters(getUpServerId(masterVolume.getClusterId()),
 sessionId, masterVolume.getName()));
Is null check for masterVolume required ?
Line 28:         
getQueryReturnValue().setReturnValue((List<GlusterGeoRepSessionConfiguration>)returnValue.getReturnValue());
Line 29:     }
Line 30: 


http://gerrit.ovirt.org/#/c/35999/5/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/ResetDefaultGeoRepConfigCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/ResetDefaultGeoRepConfigCommand.java:

Line 43:     }
Line 44: 
Line 45:     @Override
Line 46:     protected boolean canDoAction() {
Line 47:         setGlusterVolumeId(getGeoRepSession().getMasterVolumeId());
Shouldn't the volumeId be set in constructor only?
Line 48:         return super.canDoAction();
Line 49:     }
Line 50: 
Line 51:     @Override


http://gerrit.ovirt.org/#/c/35999/5/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/gluster/GlusterVolumeGeoRepSessionConfigParameters.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/gluster/GlusterVolumeGeoRepSessionConfigParameters.java:

Line 18:         this.configKey = configKey;
Line 19:         this.configValue = configValue;
Line 20:     }
Line 21: 
Line 22:     public GlusterVolumeGeoRepSessionConfigParameters(Guid volumeId, 
Guid sessionId, String configKey) {
For what purpose do we need this constructor?
Line 23:         super(volumeId, sessionId);
Line 24:         this.configKey = configKey;
Line 25:     }
Line 26: 


-- 
To view, visit http://gerrit.ovirt.org/35999
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4d98636a688f9565dff0c86c8820839cb99b1a38
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: anmolbabu <[email protected]>
Gerrit-Reviewer: Kanagaraj M <[email protected]>
Gerrit-Reviewer: Ramesh N <[email protected]>
Gerrit-Reviewer: Sahina Bose <[email protected]>
Gerrit-Reviewer: Shubhendu Tripathi <[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