Change in vdsm[master]: stomp: parsing buffer refactoring
piotr.kliczewski at gmail.com
piotr.kliczewski at gmail.com
Thu Mar 12 13:17:04 UTC 2015
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.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