Alon Bar-Lev has posted comments on this change. Change subject: engine: Get Gluster Servers query ......................................................................
Patch Set 10: (2 inline comments) For some reason these were not submitted, sorry. If you like I can just go ahead and fix it. .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GetGlusterServersQuery.java Line 93: } Line 94: } Line 95: } Line 96: Line 97: private void validateFingerprint(SSHClient client) { You can put each if block within its own function... each code block within own function... For every if you add function heder, and function suffix, your code grow exponential. It is sense to add a new function to break large chunk of codes even if it is used only once. It is sense to add a new function for code reuse. Not that I will reject this function, only that I really don't understand why it is required. Anyway, if you going to keep it, I expect you add parameter of which fingerprint to validate, move the getParameters().getFingerprint() to the place you call the function. Line 98: if (!getParameters().getFingerprint().equals(getFingerprint(client))) { Line 99: String errMsg = Line 100: "SSH Fingerprint of server " + getParameters().getServerName() Line 101: + " did not match expected fingerprint " + getParameters().getFingerprint(); Line 131: client.setUser("root"); Line 132: client.setPassword(getParameters().getRootPassword()); Line 133: Line 134: try { Line 135: client.authenticate(); Harm - no. But unexpected? yes. This discussion shows that... :) Please move out the authenticate. 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