Change in vdsm[master]: stomp: parsing buffer refactoring
fromani at redhat.com
fromani at redhat.com
Thu Mar 12 13:09:43 UTC 2015
Francesco Romani has posted comments on this change.
Change subject: stomp: parsing buffer refactoring
......................................................................
Patch Set 1: Code-Review-1
(4 comments)
mostly questions, hence -1 for visibility, but quite some of them
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 in python.
Which is the reason for this change?
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.
The convention is that we name '_' for field we don't care about, but here we do!
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?
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 _private?
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.kliczewski at gmail.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list