Alon Bar-Lev has posted comments on this change. Change subject: bootstrap: new implementation for apache-sshd usage ......................................................................
Patch Set 4: (11 inline comments) .................................................... File backend/manager/modules/utils/pom.xml Line 156: <excludes> Done .................................................... File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/hostinstall/MinaInstallWrapper.java Line 28: private static Log log = LogFactory.getLog(MinaInstallWrapper.class); Didn't touch this. Line 77: private boolean _doConnect(String server, Credentials creds) { private functions should be distinct clearly. Line 146: log.error(e); Well, as this is truly a debug mean and nothing can be done, I don't think any message will help... But added. Line 178: String tmp_fingerprint = OpenSSHUtils.getKeyFingerprintString(serverKey); Done Line 217: public void finalize() throws Exception { Both of you, we should guard developers from them-selves. If one forget for some reason to call disconnect, we have the possibility to clean up using the gc. It is far from being bad practice, and it may save us from leaking or keeping resources busy. Line 307: log.error(e); Done Line 329: public void finallize() { Again, same as above. And thanks for noting the spelling, I forgot this one. Line 340: log.error(e); Done .................................................... File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ssh/SSHClient.java Line 77: throw new IllegalArgumentException("File name should not contain \"'\""); This class is infrastructure only, the log is produce from the application (user) side. I added debug which was a valid request, but low level classes should not write into logs. Line 474: log.debug(e); No, the application should handle this, we will appear in the stack dump. For debugging purposes I added this one as well, which is unnecessary, but exists. -- To view, visit http://gerrit.ovirt.org/6722 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I50ba60f2db364114907485da3074feb714615e0c Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com> Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com> Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Ofer Schreiber <oschr...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches