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