Piotr Kliczewski has posted comments on this change. Change subject: stomp: Stomp encoder\decoder improvements ......................................................................
Patch Set 12: (3 comments) https://gerrit.ovirt.org/#/c/35255/12/lib/yajsonrpc/stomp.py File lib/yajsonrpc/stomp.py: Line 77: _heartBeatFrame = _HeartBeatFrame() Line 78: Line 79: Line 80: class Frame(object): Line 81: __slots__ = ("headers", "command", "body") > this does not belong to this patch. it's an unrelated performance optimizat Will move to separate patch. Line 82: Line 83: def __init__(self, command="", headers=None, body=None): Line 84: self.command = command Line 85: if headers is None: Line 117: def copy(self): Line 118: return Frame(self.command, self.headers.copy(), self.body) Line 119: Line 120: Line 121: def decodeValue(s): > the encodeValue/decodeValue functions just beg to have a unit test, making Done Line 122: # Make sure to leave this check before decoding as ':' can appear in the Line 123: # value after decoding using \c Line 124: if ":" in s: Line 125: raise ValueError("Contains illigal charachter `:`") Line 138: def encodeValue(s): Line 139: if isinstance(s, unicode): Line 140: s = s.encode('utf-8') Line 141: elif not isinstance(s, str): Line 142: s = str(s) > why do we want this? it makes encodeValue non-reversible. Normally, I'd rat Will improve Line 143: Line 144: return _RE_ENCODE_CHARS.sub(lambda m: _EC_ENCODE_MAP[m.group(0)], s) Line 145: Line 146: -- To view, visit https://gerrit.ovirt.org/35255 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I341e46463e654ae0a086cd49af18aa336b86cb6e Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi <smizr...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Dima Kuznetsov <dkuzn...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org 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