Dan Kenigsberg has posted comments on this change.

Change subject: netlink: event monitor with timeout
......................................................................


Patch Set 3: Code-Review-1

(5 comments)

http://gerrit.ovirt.org/#/c/37129/3/lib/vdsm/netlink/monitor.py
File lib/vdsm/netlink/monitor.py:

Line 67:         if foo:
Line 68:             mon.stop()
Line 69:         handle event
Line 70: 
Line 71:     Monitoring events with defined timeout. It there is no time left,
'It there is no time left' -> if timeout expires during iteration,

MonitorError(E_TIMEOUTED)

BTW, E_TIMEOUT looks better.
Line 72:     MonitorError: E_TIMEOUTED is raised by iteration:
Line 73:     mon = Monitor(timeout=2)
Line 74:     mon.start()
Line 75:     for event in mon:


Line 74:     mon.start()
Line 75:     for event in mon:
Line 76:         handle event
Line 77: 
Line 78:     Monitor defined groups (monitor everything if not set):
why did you drop the nice "groups" example?
Line 79:     mon = Monitor()
Line 80:     mon.start()
Line 81:     for event in mon:
Line 82:         if foo:


Line 85:     Possible groups: link, notify, neigh, tc, ipv4-ifaddr, ipv4-mroute,
Line 86:     ipv4-route ipv6-ifaddr, ipv6-mroute, ipv6-route, ipv6-ifinfo,
Line 87:     decnet-ifaddr, decnet-route, ipv6-prefix
Line 88:     """
Line 89:     def __init__(self, groups=frozenset(), timeout=None):
please add a test for the new functionality or at least replace the 
timed_monitor with it.
Line 90:         self._time_start = None
Line 91:         self._timeout = timeout
Line 92:         self._stopped = False
Line 93:         if groups:


Line 108:             yield event
Line 109: 
Line 110:     def start(self):
Line 111:         if self._timeout:
Line 112:             self._time_start = time.time()
please use monotonic_time(). setting _end_time() is slightly cleaner.
Line 113:         self._scan_thread.start()
Line 114:         self._scanning_started.wait()
Line 115: 
Line 116:     def _scan(self):


Line 127:                                 self._queue.put(_TIMEOUT_FLAG)
Line 128:                                 break
Line 129: 
Line 130:                         events = NoIntrPoll(epoll.poll, 
timeout=timeout)
Line 131:                         if events == []:
len(events) == 0

is a bit nicer since it does not assume event isn't a tuple.
Line 132:                             self._queue.put(_TIMEOUT_FLAG)
Line 133:                             break
Line 134:                         elif (self._pipetrick[0], select.POLLIN) in 
events:
Line 135:                             NoIntrCall(os.read, self._pipetrick[0], 1)


-- 
To view, visit http://gerrit.ovirt.org/37129
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If5a00814d8b7d9c4c7cba52c81b6e56f50dcc584
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to