Piotr Kliczewski has posted comments on this change. Change subject: stomp: parsing buffer refactoring ......................................................................
Patch Set 1: (4 comments) https://gerrit.ovirt.org/#/c/38666/1/lib/yajsonrpc/stomp.py File lib/yajsonrpc/stomp.py: Line 165: def _flush(self): Line 166: self._buffer = "" Line 167: Line 168: def _write_buffer(self, buff): Line 169: self._buffer += buff > so we are going to append data to a string, which is usually bad practice i The reason is not to use cStringIO. It seems that the performance of it is not needed since we we do not use so large strings. Line 170: Line 171: def _get_buffer(self): Line 172: return self._buffer Line 173: Line 172: return self._buffer Line 173: Line 174: def _handle_terminator(self, term): Line 175: res, _, rest = self._buffer.partition(term) Line 176: if not _: > well, if we do logic on this field, then we should give a name to it. Will use a name instead of '_' Line 177: return None Line 178: Line 179: self._buffer = rest Line 180: Line 396: Line 397: class AsyncDispatcher(object): Line 398: log = logging.getLogger("stomp.AsyncDispatcher") Line 399: Line 400: def __init__(self, frameHandler, bufferSize=4096): > ok, but why? Only for performance? I think performance is the reason. Line 401: self._frameHandler = frameHandler Line 402: self._bufferSize = bufferSize Line 403: self._parser = Parser() Line 404: self._outbox = deque() Line 573: Line 574: return subscriptionID Line 575: Line 576: Line 577: class Subscription(object): > is this used elsewhere outside this module? If not, what about making it _p Good idea. Even in my subscription changes this class is not used outside of this module. Line 578: def __init__(self, client, subid, ack): Line 579: self._ack = ack Line 580: self._subid = subid Line 581: self._client = client -- To view, visit https://gerrit.ovirt.org/38666 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1b835e0d8a5ca20e67f0562955255ba1ad6ae9a1 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Saggi Mizrahi <smizr...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches