Kanagaraj M has posted comments on this change.

Change subject: engine: Get Gluster Servers query
......................................................................


Patch Set 13: (1 inline comment)

Reply to Alon's comment.

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GetGlusterServersQuery.java
Line 95:                 int state = getIntValue(firstHostElement, STATE);
Line 96:                 // Add the server only if the state is 3
Line 97:                 if (state == PEER_IN_CLUSTER) {
Line 98:                     String hostName = getTextValue(firstHostElement, 
HOST_NAME);
Line 99:                     fingerprints.put(hostName, 
getFingerprint(hostName));
I think either empty or null would be fine, "N/A" would not be the correct one 
as fingerprint is always applicable in our case.  May be we can do some thing 
like, if we are not able to fetch the fingerprint, we can send 
"ERROR:reason..". So the UI checks for "ERROR" prefix, and displays the error 
reason to the user. I am not for this approach unless it adds more value to the 
user and more of hack. I mean the error message should be more specific.

The other approach would be, instead of simply having a String, we can have an 
another entity itself(Map<String, SSHFingerprintEntity>), the fields of the 
entity would be returnCode, errorMessage, fingerprint. So the UI checks the 
return code and if that is not 0, it will display whatever is there in 
errorMessage. 

The status of the server can be derived from fingerprint itself, if we are able 
to fetch the fingerprint, that means host is Up and Reachable, if we are not 
able to get it, either Host is down, sshd down or  network issue, we can say 
Host is Unreachable.
Line 100:                 }
Line 101:             }
Line 102:         }
Line 103:         return fingerprints;


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic69a9a48bf227c8fa805c8aa9c4f08fa36ea9425
Gerrit-PatchSet: 13
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dhandapani Gopal <dgo...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Dhandapani Gopal <dgo...@redhat.com>
Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com>
Gerrit-Reviewer: Michael Kublin <mkub...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Selvasundaram <sesub...@redhat.com>
Gerrit-Reviewer: Shireesh Anjal <san...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to