Piotr Kliczewski has posted comments on this change.

Change subject: asyncore: heartbeat fix
......................................................................


Patch Set 1:

(2 comments)

https://gerrit.ovirt.org/#/c/38988/1/lib/yajsonrpc/betterAsyncore.py
File lib/yajsonrpc/betterAsyncore.py:

Line 202: 
Line 203:         self._map.clear()
Line 204: 
Line 205:     def _get_timeout(self, map):
Line 206:         timeout = 30.0
> If this is the asyncore timeout, can you maybe import the constant from the
This value is default value of timeout parameter and there is no constants that 
we can use.
Line 207:         for disp in self._map.values():
Line 208:             if hasattr(disp, "next_check_interval"):
Line 209:                 interval = disp.next_check_interval()
Line 210:                 if interval is not None and interval >= 0:


Line 202: 
Line 203:         self._map.clear()
Line 204: 
Line 205:     def _get_timeout(self, map):
Line 206:         timeout = 30.0
> Maybe this should configurable or not passed at all if None?
None is problematic here. That is why we had to change it. Usually timeout says 
how long asyncore loop is waiting for events if the timeout passes we check 
intervals and wait for events again. I could be any value here when we do not 
care but using asyncore default seems to be the best approach.
Line 207:         for disp in self._map.values():
Line 208:             if hasattr(disp, "next_check_interval"):
Line 209:                 interval = disp.next_check_interval()
Line 210:                 if interval is not None and interval >= 0:


-- 
To view, visit https://gerrit.ovirt.org/38988
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icad61fb62509764e8ecdd6f87ef60557f4465368
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczew...@gmail.com>
Gerrit-Reviewer: Dima Kuznetsov <dkuzn...@redhat.com>
Gerrit-Reviewer: Francesco Romani <from...@redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizr...@redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykap...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to