Change in vdsm[master]: netlink: event monitor
phoracek at redhat.com
phoracek at redhat.com
Mon Jan 19 12:56:15 UTC 2015
Petr Horáček has posted comments on this change.
Change subject: netlink: event monitor
......................................................................
Patch Set 27:
(7 comments)
http://gerrit.ovirt.org/#/c/36197/27/lib/vdsm/netlink/monitor.py
File lib/vdsm/netlink/monitor.py:
Line 75: ipv4-route ipv6-ifaddr, ipv6-mroute, ipv6-route, ipv6-ifinfo,
Line 76: decnet-ifaddr, decnet-route, ipv6-prefix
Line 77: """
Line 78: def __init__(self, groups=frozenset()):
Line 79: self._stopped = False
> should be renamed to 'stopped' as it is used (read) publically
it is meant to be used locally, tests are just an exception
Line 80: if groups:
Line 81: self._groups = groups
Line 82: else:
Line 83: self._groups = _GROUPS.keys()
http://gerrit.ovirt.org/#/c/36197/27/tests/netlinkTests.py
File tests/netlinkTests.py:
Line 15: class NetlinkEventMonitorTests(TestCaseBase):
Line 16:
Line 17: @ValidateRunningAsRoot
Line 18: def test_iterate_after_events(self):
Line 19: with _timed_monitor(1) as mon:
> just a matter of style, but passing this as a named argumet might be more r
Done
Line 20: dummy_name = dummy.create()
Line 21: dummy.remove(dummy_name)
Line 22: for event in mon:
Line 23: if event.get('name') == dummy_name:
Line 18: def test_iterate_after_events(self):
Line 19: with _timed_monitor(1) as mon:
Line 20: dummy_name = dummy.create()
Line 21: dummy.remove(dummy_name)
Line 22: for event in mon:
> Just so I understand: when no events are collected by the monitor, does the
If we did not catch dummy-related event, it fails with MonitorError, yes.
We are not able to test strictly no-events case, because we can't be sure, there will be no external event (but we can just ignore incoming events).
I think testing timeout is not necessary, because now it's just a test's feature, not a part of monitor.py.
Line 23: if event.get('name') == dummy_name:
Line 24: break
Line 25:
Line 26: @ValidateRunningAsRoot
Line 29: iteration we start _add_device, which waits for iteration_ready event.
Line 30: """
Line 31: dummy_name = dummy.create()
Line 32:
Line 33: def _add_device():
> may rename this function to _add_and_remove?
if i do dummy.create() inside the thread's function, it would be hard to pass dummy's name to the main thread.
Line 34: time.sleep(.2)
Line 35: dummy.setLinkUp(dummy_name)
Line 36: dummy.remove(dummy_name)
Line 37: add_device_thread = threading.Thread(target=_add_device)
Line 38:
Line 39: with _timed_monitor(1) as mon:
Line 40: add_device_thread.start()
Line 41: for event in mon:
Line 42: print event
> why print?
Done
Line 43: if event.get('name') == dummy_name:
Line 44: break
Line 45: add_device_thread.join()
Line 46:
Line 72: "to link or route." % event['event'])
Line 73:
Line 74: @ValidateRunningAsRoot
Line 75: def test_iteration(self):
Line 76: with _timed_monitor(1) as mon:
> * is this timeout enough even for slow Jenkins vms?
* i don't think so, the test passes within 0.038 second on vdsm-function-tests Jenkins VM
* StopIteration could happen only after the monitor is stopped. If it is monitoring, iterator.next() stuck until there is an event available.
Line 77: iterator = iter(mon)
Line 78:
Line 79: # Generate events to avoid blocking
Line 80: dummy_name = dummy.create()
Line 123: % (1 + len(expected_events)))
Line 124:
Line 125:
Line 126: @contextmanager
Line 127: def _timed_monitor(time, groups=frozenset(), raise_exception=True):
> why renaming this to 'time'? Maybe 'interval' (the original Timer arg name)
Done
Line 128: mon = monitor.Monitor(groups=groups)
Line 129: mon.start()
Line 130: try:
Line 131: timer = Timer(time, mon.stop)
--
To view, visit http://gerrit.ovirt.org/36197
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0f4fcfde87ad51eb832f54862371b4da1281826e
Gerrit-PatchSet: 27
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček <phoracek at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <toni at midokura.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Ondřej Svoboda <osvoboda at redhat.com>
Gerrit-Reviewer: Petr Horáček <phoracek 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