Dan Kenigsberg has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 15: (2 inline comments)
.................................................... File vdsm_api/BindingJsonRpc.py Line 37: self._reactors = reactors Line 38: Line 39: def _createTcpReactor(self, cfg): Line 40: address = cfg.get("ip", "0.0.0.0") Line 41: port = cfg.get("port", 4044) sorry, I do not follow. which error handling code would have been required? to handle the case of not having cfg['ip'] ? having a default here only hide such an error instead of crying out loud about it. Line 42: return TCPReactor((address, port), self.server) Line 43: Line 44: def start(self): Line 45: for reactor in self._reactors:
.................................................... File vdsm_api/jsonrpc/tcpReactor.py Line 63: buffLen = len(self._buffer) Line 64: if buffLen < _Size.size: Line 65: return None Line 66: Line 67: if buffLen > self.buffsize: I would not forget Vinzenz's +1s. do you have other people acks? I know that I haven't looked beyond this first patch. Line 68: self._conn.close() Line 69: self._sendData = [] Line 70: Line 71: msgLen = _Size.unpack(self._buffer[:_Size.size])[0]
-- To view, visit http://gerrit.ovirt.org/9986 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I7a65364824cc8433263ccf6ed44f1f97141eff2c Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server