Douglas Schilling Landgraf 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. I was running away from it but I am ok as well to use static "/config" in os.path.exists to check if file is persisted. 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. Agreed 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 != "Non If you don't mind I prefer to stay with this cfg_fprint. If we don't have cfg_fprint no need to use _compare_fingeprint 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: We need the download_certificate_succeeded otherwise 80 port will achieve here because of all fOK until here. 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: 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
