Shireesh Anjal has posted comments on this change.

Change subject: gluster: Use brick server for advanced details
......................................................................


Patch Set 1: (3 inline comments)

responses to comments in-line. new patch-set to follow.

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GetGlusterVolumeAdvancedDetailsQuery.java
Line 35:         super(params);
Line 36:     }
Line 37: 
Line 38:     @Override
Line 39:     protected void executeQueryCommand() {
Sorry, can't really do this. Because the testing framework mocks both the query 
and the parameters, moving these lines to the constructors starts failing my 
tests.

There might be a way to make it work by moving these to constructor, but I 
think there is no harm in keeping these here as well, as executeQueryCommand() 
is the entry point for the functionality, and there is no chance that these 
variables don't get populated.

So will keep this code as is.
Line 40:         clusterId = getParameters().getClusterId();
Line 41:         detailRequired = getParameters().isDetailRequired();
Line 42:         volumeId = getParameters().getVolumeId();
Line 43:         if (volumeId != null) {


Line 110:      * Otherwise returns a random up server from the cluster
Line 111:      */
Line 112:     protected Guid getUpServerId() {
Line 113:         if (brick == null) {
Line 114:             return super.getUpServerId(clusterId);
Done
Line 115:         }
Line 116: 
Line 117:         VDS brickServer = getVdsDao().get(brick.getServerId());
Line 118:         if (brickServer.getStatus() == VDSStatus.Up) {


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterQueriesCommandBase.java
Line 22:         super(parameters);
Line 23:     }
Line 24: 
Line 25:     protected VdsDAO getVdsDao() {
Line 26:         return DbFacade.getInstance().getVdsDao();
Will do. Note that I'll have to make it public as my class is in a different 
(gluster) package.
Line 27:     }
Line 28: 
Line 29:     protected GlusterVolumeDao getGlusterVolumeDao() {
Line 30:         return DbFacade.getInstance()


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4acb0bbe84bacab714926d69b639bbda970df9a
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Shireesh Anjal <san...@redhat.com>
Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Sahina Bose <sab...@redhat.com>
Gerrit-Reviewer: Shireesh Anjal <san...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to