Yaniv Bronhaim has posted comments on this change. Change subject: Have the protocol detector use the Reactor ......................................................................
Patch Set 3: Code-Review-1 (5 comments) https://gerrit.ovirt.org/#/c/37098/3/lib/yajsonrpc/betterAsyncore.py File lib/yajsonrpc/betterAsyncore.py: Line 67: try: Line 68: impl.init(self) Line 69: except AttributeError: Line 70: # impl.init() is optional. Line 71: pass use hasattr for that .. Line 72: Line 73: self._bind_implementation() Line 74: Line 75: def next_check_interval(self): https://gerrit.ovirt.org/#/c/37098/3/tests/protocoldetectorTests.py File tests/protocoldetectorTests.py: Line 36: """ Line 37: Acceptor with shorter cleanup interval to make it practical to test the Line 38: cleanup logic. Line 39: """ Line 40: CLEANUP_INTERVAL = 1.0 looks like nobody uses it anymore, please remove Line 41: Line 42: Line 43: class Detector(object): Line 44: """ Line 206: Line 207: def start_acceptor(self, use_ssl): Line 208: self.acceptor = TestingAcceptor( Line 209: '127.0.0.1', 0, sslctx=self.SSLCTX if use_ssl else None) Line 210: self.acceptor._handshake_timeout = 1 why not using the constant .. easier to configure if needed Line 211: self.acceptor.add_detector(Echo()) Line 212: self.acceptor.add_detector(Uppercase()) Line 213: self.acceptor_address = self.acceptor._acceptor.socket.getsockname() Line 214: t = threading.Thread(target=self.acceptor.serve_forever) https://gerrit.ovirt.org/#/c/37098/3/vdsm/protocoldetector.py File vdsm/protocoldetector.py: Line 33: ) Line 34: Line 35: from vdsm.sslutils import ( Line 36: SSLHandshakeWrapper, Line 37: ) why brackets? new saggi's style? Line 38: Line 39: Line 40: def _create_socket(host, port): Line 41: addr = socket.getaddrinfo(host, port, socket.AF_INET, Line 73: client, _ = dispatcher.socket.accept() Line 74: except socket.error: Line 75: pass Line 76: Line 77: client.setblocking(0) you pass on exception.. client can be None Line 78: self.log.info("Accepting connection from %s:%d", *client.getpeername()) Line 79: self._dispatcher_factory(client) Line 80: Line 81: -- To view, visit https://gerrit.ovirt.org/37098 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If1cc2a205cdad335e8c3af4a77ed49c8977a79ba Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
