Yaniv Bronhaim has posted comments on this change. Change subject: reactor: using single instance ......................................................................
Patch Set 5: -Code-Review (2 comments) https://gerrit.ovirt.org/#/c/40170/5/vdsm/clientIF.py File vdsm/clientIF.py: Line 110: self.threadLocal.client = '' Line 111: Line 112: host = config.get('addresses', 'management_ip') Line 113: port = config.getint('addresses', 'management_port') Line 114: self._createReactor() why not just self._reactor = Reactor() or doing that inside the createAcceptor? this constructor is quite big anyway - you can centralize this inside the createAcceptor which I would call initProtocolDetector instead of adding more new buzzwords - on start and stop I'd check if self._reactor is none to avoid coding mistakes Line 115: self._createAcceptor(host, port) Line 116: self._prepareXMLRPCBinding() Line 117: self._prepareJSONRPCBinding() Line 118: except: Line 242: self._acceptor.stop() Line 243: for binding in self.bindings.values(): Line 244: binding.stop() Line 245: Line 246: self._reactor.stop() fyi, the "acceptor".stop (MultiProtocolAcceptor.stop) calls to its reactor stop already - it doesn't cause any harm but better to be aware Line 247: Line 248: self._enabled = False Line 249: self.channelListener.stop() Line 250: self._hostStats.stop() -- To view, visit https://gerrit.ovirt.org/40170 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8f36ec36b91fcc46e1ab46eb92f9739e9e19e448 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Dima Kuznetsov <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: Yeela Kaplan <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
