Alon Bar-Lev has posted comments on this change. Change subject: vdsm-upgrade: adds wrapper to ovirt-node-upgrade ......................................................................
Patch Set 11: (2 comments) great, last minor comments. http://gerrit.ovirt.org/#/c/28244/11/vdsm_reg/vdsm-upgrade File vdsm_reg/vdsm-upgrade: Line 15: Line 16: from xml.sax import saxutils Line 17: Line 18: Line 19: def _output(status, component, msg_line=None): status can be success/boolean, so you put 'FAIL' or 'OK' here inside. s/message_line/message/ Line 20: """ Line 21: Encapsulate the message into XML for Engine Line 22: Line 23: Args: Line 79: component='ovirt-node-upgrade', Line 80: msg_line=line Line 81: ) Line 82: sys.stdout.write(msg) Line 83: sys.stdout.flush() why not move the write/flush/debug into the output? Line 84: logging.debug(msg) Line 85: Line 86: if upgrade_tool.returncode == 0: Line 87: msg = _output( -- To view, visit http://gerrit.ovirt.org/28244 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7b997d70a440545497246d1a19d9671b054a56a5 Gerrit-PatchSet: 11 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: Joey Boggs <[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
