Alon Bar-Lev has posted comments on this change. Change subject: vdsm_config: move download certificate ......................................................................
Patch Set 26: (4 comments) http://gerrit.ovirt.org/#/c/26718/26/vdsm_reg/vdsm-reg-setup.in File vdsm_reg/vdsm-reg-setup.in: Line 213: "Engine fingerprint [%s]", Line 214: self.cfg_fprint.upper(), Line 215: engine_fprint Line 216: ) Line 217: if File(ca_path).exists(): os.path.exists() should be used these days, I think. most probably there is no sense to call generateFingerPrint if file does not exist, so condition should be on outer scope. Line 218: ovirtfunctions.ovirt_safe_delete_config( Line 219: ca_path Line 220: ) Line 221: os.unlink(ca_path) Line 217: if File(ca_path).exists(): Line 218: ovirtfunctions.ovirt_safe_delete_config( Line 219: ca_path Line 220: ) Line 221: os.unlink(ca_path) this should be indented, no reason to unlink if not exists. Line 222: Line 223: return ret Line 224: Line 225: def execute(self): Line 234: fOK = self.renameBridge() Line 235: logging.debug("execute: after renameBridge: %s", fOK) Line 236: Line 237: if fOK and os.path.exists(self.engineWebCACert) and \ Line 238: self.cfg_fprint != "None": move the "None" into the function, always return true and drop this != "None" condition. Line 239: self._compare_fingerprint(self.engineWebCACert) Line 240: Line 241: if fOK and not os.path.exists(self.engineWebCACert): Line 242: fd_tmp, file_tmp = tempfile.mkstemp() Line 259: ) Line 260: fOk = False Line 261: Line 262: if fOK and self.cfg_fprint != "None" and \ Line 263: download_certificate_succeeded: if you move the "None" to function all it should be should be: if fOK: but even if not... this condition should be: if fOK and self.cfg_fprint != "None": Line 264: if not self._compare_fingerprint(file_tmp): Line 265: fOK = False Line 266: sys.exit(1) Line 267: -- To view, visit http://gerrit.ovirt.org/26718 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia2c025eedd2be92b9418ddbe01efc02c913af2a7 Gerrit-PatchSet: 26 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Douglas Schilling Landgraf <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
