Francesco Romani has posted comments on this change. Change subject: asyncore: dynamic tick support ......................................................................
Patch Set 8: (2 comments) nice, but being too terse it is harder to understand than it could. Suggested possible improvements inside https://gerrit.ovirt.org/#/c/37057/8/lib/yajsonrpc/betterAsyncore.py File lib/yajsonrpc/betterAsyncore.py: Line 81: return getattr( Line 82: self.__impl, Line 83: "next_check_interval", Line 84: lambda self: None Line 85: )(self) the combined usage of lambda and getattr made this snippet hard to process. It is just too terse. To improve, we could - _require_ the next_check_interval to be present and/or - expand this code to be a bit more verbose and easy to follow Line 86: Line 87: def recv(self, buffer_size): Line 88: try: Line 89: data = self.socket.recv(buffer_size) Line 168: for disp in self._map.itervalues(): Line 169: if hasattr(disp, "next_check_interval"): Line 170: interval = disp.next_check_interval() Line 171: if interval is not None and interval >= 0: Line 172: timeout = min(interval, timeout) this for loop above could be extracted ina function which will make it more testable and readable (we can add docstring and a describing name). That could made the code a bit slower (functions calls aren't cheap in python :( ) but IMO this is a price which is worth paying here. Line 173: Line 174: asyncore.loop( Line 175: timeout=timeout, Line 176: use_poll=True, -- To view, visit https://gerrit.ovirt.org/37057 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I44b38e961d46e914bb687c924ba4e83f38371d5b Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: Yeela Kaplan <[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
