Nir Soffer has posted comments on this change. Change subject: logging: Don't crash on non-ASCII in SimpleLogAdapter ......................................................................
Patch Set 1: (1 comment) https://gerrit.ovirt.org/#/c/48542/1/tests/utilsTests.py File tests/utilsTests.py: Line 1084: # various codings. Line 1085: invalid = chr(254) + chr(0) Line 1086: u_invalid = unichr(253) + unichr(254) Line 1087: extra = dict(e1=u'Hello', e2='Здра́вствуйте', e3=u'שלום', e4='你好', Line 1088: e5=invalid, e6=u_invalid) > Believe me or not, vm_id as passed to SimpleLogAdapter constructor in Vm.__ vm id may is unicode string like this: u"474ed6cc-e085-469c-b779-f9b036d96931" This happens because we use json to pass the parameters, and json parse everything to unicode. This is however unicode instance containing ascii characters, and it does not need any special handling. The correct fix for this is to do: self.log = SimpleLogAdapter(self.log, {"vmId": str(self.conf['vmId'])}) In vm.py. So SimpleLogAdapter does not need to handle unicode in the extra dict. Line 1089: log = logUtils.SimpleLogAdapter(self._log, extra) Line 1090: log.debug("Dobrý večer") Line 1091: log.debug(u"Dobrý večer") Line 1092: log.debug(invalid) -- To view, visit https://gerrit.ovirt.org/48542 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1930817ade1799b218913348ffa099aab2211ef7 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal <mzama...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Milan Zamazal <mzama...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
