Alon Bar-Lev has posted comments on this change.

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


Patch Set 10: (1 inline comment)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GetGlusterServersQuery.java
Line 131:         client.setUser("root");
Line 132:         client.setPassword(getParameters().getRootPassword());
Line 133: 
Line 134:         try {
Line 135:             client.authenticate();
For the SSHClient exceptions - if I wrap each exception within RuntimeException 
we will not be able to couch these specific exceptions.

There is no problem in one catch at command level to wrap RuntiemException.

For the authenticate... the fact that it has "And" in function name shows that 
it does not do one logical task as you stated above.

Why won't we have connectAndValidate()? or connectValidateAndAuthenicate? or 
connectValidateAuthenticateAndExecite()?

I guess I would not argue so much if I could imagine similar pattern that will 
allow running one command.

Notice that this function is a complete void... tree line function... I think I 
will submit a different patch collapsing the one line functions into one flow, 
just for you to see.
Line 136:             client.executeCommand(command, null, out, null);
Line 137:             cliOutput = new String(out.toByteArray(), "UTF-8");
Line 138:         } catch (Exception e) {
Line 139:             String m =


--
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: 10
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