Alon Bar-Lev has posted comments on this change.
Change subject: tools: Adds notifier config validation prior to service startup
......................................................................
Patch Set 4:
(4 comments)
....................................................
File packaging/services/ovirt-engine-notifier/ovirt-engine-notifier.py
Line 204: stdout, stderr = proc.communicate()
Line 205:
Line 206: if stdout:
Line 207: self.logger.info(
Line 208: 'validation: %s',
all texts that goes to user should be gettext:
Line 209: stdout.decode('utf-8', 'replace'),
Line 210: )
Line 211:
Line 212: if proc.returncode != 0:
Line 210: )
Line 211:
Line 212: if proc.returncode != 0:
Line 213: if stderr:
Line 214: stderr = stderr.decode('utf-8', 'replace').rstrip()
I think that stdout and stderr are equals and should be printed to log if exist
regardless of return code.
Line 215: raise RuntimeError(stderr)
Line 216:
Line 217:
Line 218: if __name__ == '__main__':
Line 211:
Line 212: if proc.returncode != 0:
Line 213: if stderr:
Line 214: stderr = stderr.decode('utf-8', 'replace').rstrip()
Line 215: raise RuntimeError(stderr)
please remove this in favor of generic error message: _('Configuration is
invalid') ot something similar.
Line 216:
Line 217:
Line 218: if __name__ == '__main__':
Line 219: service.setupLogger()
Line 212: if proc.returncode != 0:
Line 213: if stderr:
Line 214: stderr = stderr.decode('utf-8', 'replace').rstrip()
Line 215: raise RuntimeError(stderr)
Line 216:
Summary, something similar to:
if stdout or stderr:
def printstd(std, prefix):
if std:
for l in std.decode('utf-8', 'replace').splitlines():
self.logger.info('%s: %s', prefix, l)
self.logger.info(_('Validation result:'))
printstd(stdout, 'stdout')
printstd(stderr, 'stderr')
if proc.returncode != 0:
self.logger.error(
_('Validation failed returncode is {rc}:').format(
rc=proc.returncode,
)
)
raise RuntimeError(_('Configuration is invalid'))
Line 217:
Line 218: if __name__ == '__main__':
Line 219: service.setupLogger()
Line 220: d = Daemon()
--
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: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Peřina <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Martin Peřina <[email protected]>
Gerrit-Reviewer: Sandro Bonazzola <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: mooli tayer <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
