Change in vdsm[master]: asyncore: dynamic tick support
fromani at redhat.com
fromani at redhat.com
Wed Mar 4 16:39:54 UTC 2015
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 <smizrahi at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list