Yaniv Bronhaim has posted comments on this change. Change subject: Unified network persistence [4.1/4.*] - Upgrade mechanism ......................................................................
Patch Set 13: Code-Review-1 (5 comments) Looks like all (or at least most) of this patch is about the logger that I don't want .................................................... Commit Message Line 9: Unified network persistence feature: Line 10: http://www.ovirt.org/Feature/NetworkReloaded#Unified_persistence Line 11: Line 12: This patch is the first of two patches that implement an upgrade Line 13: that is the ran the first time VDSM that uses unified network remove the "the" Line 14: persistence is started. The goal is to look at the current Line 15: host networking configuration and populate the running config. Line 16: Line 17: Please see the next patch in the series: Line 16: Line 17: Please see the next patch in the series: Line 18: http://gerrit.ovirt.org/#/c/18425/ Line 19: Line 20: To better understand how this patch is used. the link to the wiki is enough .. you don't need to mention the next patch imo Line 21: Line 22: Change-Id: Iba3c9c34f03134c192db1c2add31084824e195d9 .................................................... File lib/vdsm/tool/upgrade.py Line 22: import os Line 23: Line 24: from vdsm import constants Line 25: Line 26: LOGGER_CONF_FILE = constants.P_VDSM_CONF + 'vdsm-tool.logger.conf' Please don't add this logger. I don't see the reason for it I don't mind if you want to submit it in separate patch and chat about it there Line 27: Line 28: Line 29: class Upgrade(object): Line 30: def __init__(self, upgradeName): Line 45: def seal(self): Line 46: """ Line 47: Mark the upgrade as a success Line 48: """ Line 49: self.log.debug("Upgrade %s successfully performed" % self.upgradeName) use sys.stdout.write for that Line 46: """ Line 47: Mark the upgrade as a success Line 48: """ Line 49: self.log.debug("Upgrade %s successfully performed" % self.upgradeName) Line 50: open(self.upgradeFilePath, 'w').close() can't be exception in this line? like permission errors and etc.. -- To view, visit http://gerrit.ovirt.org/17726 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba3c9c34f03134c192db1c2add31084824e195d9 Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]> Gerrit-Reviewer: Assaf Muller <[email protected]> Gerrit-Reviewer: Barak Azulay <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Giuseppe Vallarelli <[email protected]> Gerrit-Reviewer: Mark Wu <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[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
