Alon Bar-Lev has posted comments on this change.
Change subject: tools: Adds notifier config validation prior to service startup
......................................................................
Patch Set 8:
(4 comments)
ok, so now we are feature complete!
few more annoying comments.
....................................................
File packaging/services/ovirt-engine-notifier/ovirt-engine-notifier.py
Line 204:
Line 205: stdout, stderr = proc.communicate()
Line 206:
Line 207: def printToBoth(msg):
Line 208: self.logger.debug(msg)
if msg contain % it will fail, should be:
self.logger.debug('%s', msg)
Line 209: sys.stderr.write(msg)
Line 210: sys.stderr.write('\n')
Line 211:
Line 212: def printstd(std, prefix):
Line 206:
Line 207: def printToBoth(msg):
Line 208: self.logger.debug(msg)
Line 209: sys.stderr.write(msg)
Line 210: sys.stderr.write('\n')
no need for two...
sys.stderr.write(msg + '\n')
Line 211:
Line 212: def printstd(std, prefix):
Line 213: if std:
Line 214: for l in std.decode('utf-8', 'replace').splitlines():
Line 212: def printstd(std, prefix):
Line 213: if std:
Line 214: for l in std.decode('utf-8', 'replace').splitlines():
Line 215: msg = '%s: %s' % (prefix, l)
Line 216: printToBoth(msg)
no need for msg temp variable
Line 217:
Line 218: if stdout or stderr:
Line 219: printToBoth(_('\nValidation result:'))
Line 220: printstd(stdout, 'stdout')
Line 215: msg = '%s: %s' % (prefix, l)
Line 216: printToBoth(msg)
Line 217:
Line 218: if stdout or stderr:
Line 219: printToBoth(_('\nValidation result:'))
drop the \n here, it is not good for our logging, and I am unsure it is
required.
Line 220: printstd(stdout, 'stdout')
Line 221: printstd(stderr, 'stderr')
Line 222:
Line 223: if proc.returncode != 0:
--
To view, visit http://gerrit.ovirt.org/22629
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I11ad77a33697f254e60e1d0b12e343900e5b0b34
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Peřina <mper...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Martin Peřina <mper...@redhat.com>
Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: mooli tayer <mta...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
