Yaniv Bronhaim has posted comments on this change.
Change subject: Unified network persistence [4.1/4.*] - Upgrade mechanism
......................................................................
Patch Set 10:
(1 comment)
....................................................
File lib/vdsm/tool/upgrade.py
Line 36: self.log = logging.getLogger('vdsm-tool')
Line 37: except Exception:
Line 38: logging.basicConfig(filename='/dev/stdout', filemode='w+',
Line 39: level=logging.DEBUG)
Line 40: logging.error("Could not init proper logging",
exc_info=True)
could be nice to have logging though, but you should report to stderr anyway,
so the user will know about the fail, or raise RuntimeError
logging can be nice alright, but not needed in a tool.. its like having a
logger to vdsClient instead of printing out the errors. where will you use it?
all stderr will be flushed to /var/log/messages, that's enough as it seems to me
Line 41:
Line 42: def isNeeded(self):
Line 43: return not os.path.exists(self.upgradeFilePath)
Line 44:
--
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: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Assaf Muller <amul...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@redhat.com>
Gerrit-Reviewer: Assaf Muller <amul...@redhat.com>
Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com>
Gerrit-Reviewer: Giuseppe Vallarelli <gvall...@redhat.com>
Gerrit-Reviewer: Mark Wu <wu...@linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizr...@redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
