qubes/events: they accept only keyword arguments

Positional arguments are hereby deprecated, with immediate effect.

QubesOS/qubes-issues#2622

doc/qubes-events.rst

@@ -13,8 +13,9 @@ Firing events
 Events are fired by calling :py:meth:`qubes.events.Emitter.fire_event`. The
 first argument is event name (a string). You can fire any event you wish, the
 names are not checked in any way, however each class' documentation tells what
-standard events will be fired on it. The rest of arguments are dependent on the
+standard events will be fired on it. When firing an event, caller may specify
-particular event in question -- they are passed as-is to handlers.
+some optional keyword arguments. Those are dependent on the particular event in
+question -- they are passed as-is to handlers.
 
 Event handlers are fired in reverse method resolution order, that is, first for
 parent class and then for it's child. For each class, first are called handlers

qubes/__init__.py

@@ -250,19 +250,19 @@ class property(object): # pylint: disable=redefined-builtin,invalid-name
 
         if has_oldvalue:
             instance.fire_event_pre('property-pre-set:' + self.__name__,
-                self.__name__, value, oldvalue)
+                name=self.__name__, newvalue=value, oldvalue=oldvalue)
         else:
             instance.fire_event_pre('property-pre-set:' + self.__name__,
-                self.__name__, value)
+                name=self.__name__, newvalue=value)
 
         instance._property_init(self, value) # pylint: disable=protected-access
 
         if has_oldvalue:
-            instance.fire_event('property-set:' + self.__name__, self.__name__,
+            instance.fire_event('property-set:' + self.__name__,
-                value, oldvalue)
+                name=self.__name__, newvalue=value, oldvalue=oldvalue)
         else:
-            instance.fire_event('property-set:' + self.__name__, self.__name__,
+            instance.fire_event('property-set:' + self.__name__,
-                value)
+                name=self.__name__, newvalue=value)
 
 
     def __delete__(self, instance):
@@ -276,16 +276,16 @@ class property(object): # pylint: disable=redefined-builtin,invalid-name
 
         if has_oldvalue:
             instance.fire_event_pre('property-pre-del:' + self.__name__,
-                self.__name__, oldvalue)
+                name=self.__name__, oldvalue=oldvalue)
             delattr(instance, self._attr_name)
             instance.fire_event('property-del:' + self.__name__,
-                self.__name__, oldvalue)
+                name=self.__name__, oldvalue=oldvalue)
 
         else:
             instance.fire_event_pre('property-pre-del:' + self.__name__,
-                self.__name__)
+                name=self.__name__)
             instance.fire_event('property-del:' + self.__name__,
-                self.__name__)
+                name=self.__name__)
 
 
     def __repr__(self):
@@ -601,7 +601,7 @@ class PropertyHolder(qubes.events.Emitter):
             except AttributeError:
                 continue
 
-        self.fire_event('clone-properties', src, proplist)
+        self.fire_event('clone-properties', src=src, proplist=proplist)
 
 
     def property_require(self, prop, allow_none=False, hard=False):

qubes/app.py

@@ -416,7 +416,7 @@ class VMCollection(object):
         self._dict[value.qid] = value
         if _enable_events:
             value.events_enabled = True
-            self.app.fire_event('domain-add', value)
+            self.app.fire_event('domain-add', vm=value)
 
         return value
 
@@ -445,7 +445,7 @@ class VMCollection(object):
         vm = self[key]
         if not vm.is_halted():
             raise qubes.exc.QubesVMNotHaltedError(vm)
-        self.app.fire_event_pre('domain-pre-delete', vm)
+        self.app.fire_event_pre('domain-pre-delete', vm=vm)
         try:
             vm.libvirt_domain.undefine()
         except libvirt.libvirtError as e:
@@ -453,7 +453,7 @@ class VMCollection(object):
                 # already undefined
                 pass
         del self._dict[vm.qid]
-        self.app.fire_event('domain-delete', vm)
+        self.app.fire_event('domain-delete', vm=vm)
 
     def __contains__(self, key):
         return any((key == vm or key == vm.qid or key == vm.name)
@@ -1074,7 +1074,8 @@ class Qubes(qubes.PropertyHolder):
             if not vm.provides_network and vm.property_is_default('netvm'):
                 # fire property-del:netvm as it is responsible for resetting
                 # netvm to it's default value
-                vm.fire_event('property-del:netvm', 'netvm', newvalue, oldvalue)
+                vm.fire_event('property-del:netvm',
+                    name='netvm', newvalue=newvalue, oldvalue=oldvalue)
 
 
     @qubes.events.handler('property-set:default_netvm')
@@ -1085,4 +1086,5 @@ class Qubes(qubes.PropertyHolder):
             if vm.provides_network and vm.property_is_default('netvm'):
                 # fire property-del:netvm as it is responsible for resetting
                 # netvm to it's default value
-                vm.fire_event('property-del:netvm', 'netvm', oldvalue)
+                vm.fire_event('property-del:netvm',
+                    name='netvm', oldvalue=oldvalue)

qubes/devices.py

@@ -130,10 +130,10 @@ class DeviceCollection(object):
             raise DeviceAlreadyAttached(
                 'device {!r} of class {} already attached to {!r}'.format(
                     device, self._class, self._vm))
-        self._vm.fire_event_pre('device-pre-attach:' + self._class, device)
+        self._vm.fire_event_pre('device-pre-attach:'+self._class, device=device)
         if persistent:
             self._set.add(device)
-        self._vm.fire_event('device-attach:' + self._class, device)
+        self._vm.fire_event('device-attach:' + self._class, device=device)
 
 
     def detach(self, device, persistent=True):
@@ -146,10 +146,10 @@ class DeviceCollection(object):
             raise DeviceNotAttached(
                 'device {!s} of class {} not attached to {!s}'.format(
                     device, self._class, self._vm))
-        self._vm.fire_event_pre('device-pre-detach:' + self._class, device)
+        self._vm.fire_event_pre('device-pre-detach:'+self._class, device=device)
         if persistent:
             self._set.remove(device)
-        self._vm.fire_event('device-detach:' + self._class, device)
+        self._vm.fire_event('device-detach:' + self._class, device=device)
 
     def attached(self, persistent=None):
         '''List devices which are (or may be) attached to this vm
@@ -212,7 +212,7 @@ class DeviceCollection(object):
         :raises AssertionError: when multiple devices with the same ident are
         found
         '''
-        dev = self._vm.fire_event('device-get:' + self._class, ident)
+        dev = self._vm.fire_event('device-get:' + self._class, ident=ident)
         if dev:
             assert len(dev) == 1
             return dev[0]

qubes/events.py

@@ -116,7 +116,7 @@ class Emitter(object, metaclass=EmitterMeta):
         cls.__handlers__[event].add(func)
 
 
-    def _fire_event_in_order(self, order, event, *args, **kwargs):
+    def _fire_event_in_order(self, order, event, kwargs):
         '''Fire event for classes in given order.
 
         Do not use this method. Use :py:meth:`fire_event` or
@@ -136,13 +136,13 @@ class Emitter(object, metaclass=EmitterMeta):
             for func in sorted(handlers,
                     key=(lambda handler: hasattr(handler, 'ha_bound')),
                     reverse=True):
-                effect = func(self, event, *args, **kwargs)
+                effect = func(self, event, **kwargs)
                 if effect is not None:
                     effects.extend(effect)
         return effects
 
 
-    def fire_event(self, event, *args, **kwargs):
+    def fire_event(self, event, **kwargs):
         '''Call all handlers for an event.
 
         Handlers are called for class and all parent classess, in **reversed**
@@ -156,15 +156,15 @@ class Emitter(object, metaclass=EmitterMeta):
         :param str event: event identificator
         :returns: list of effects
 
-        All *args* and *kwargs* are passed verbatim. They are different for
+        All *kwargs* are passed verbatim. They are different for different
-        different events.
+        events.
         '''
 
         return self._fire_event_in_order(reversed(self.__class__.__mro__),
-            event, *args, **kwargs)
+            event, kwargs)
 
 
-    def fire_event_pre(self, event, *args, **kwargs):
+    def fire_event_pre(self, event, **kwargs):
         '''Call all handlers for an event.
 
         Handlers are called for class and all parent classess, in **true**
@@ -177,9 +177,9 @@ class Emitter(object, metaclass=EmitterMeta):
         :param str event: event identificator
         :returns: list of effects
 
-        All *args* and *kwargs* are passed verbatim. They are different for
+        All *kwargs* are passed verbatim. They are different for different
-        different events.
+        events.
         '''
 
         return self._fire_event_in_order(self.__class__.__mro__,
-            event, *args, **kwargs)
+            event, kwargs)

qubes/ext/gui.py

@@ -263,21 +263,21 @@ class GUI(qubes.ext.Extension):
 
 
     @qubes.ext.handler('monitor-layout-change')
-    def on_monitor_layout_change(self, vm, event, monitor_layout=None):
+    def on_monitor_layout_change(self, vm, event, layout=None):
         # pylint: disable=no-self-use,unused-argument
         if vm.features.check_with_template('no-monitor-layout', False) \
                 or not vm.is_running():
             return
 
-        if monitor_layout is None:
+        if layout is None:
-            monitor_layout = get_monitor_layout()
+            layout = get_monitor_layout()
-            if not monitor_layout:
+            if not layout:
                 return
 
         pipe = vm.run('QUBESRPC qubes.SetMonitorLayout dom0',
             passio_popen=True, wait=True)
 
-        pipe.stdin.write(''.join(monitor_layout))
+        pipe.stdin.write(''.join(layout))
         pipe.stdin.close()
         pipe.wait()
 

qubes/firewall.py

@@ -269,27 +269,31 @@ class Rule(qubes.PropertyHolder):
 
     # noinspection PyUnusedLocal
     @qubes.events.handler('property-pre-set:dstports')
-    def on_set_dstports(self, _event, _prop, _new_value, _old_value=None):
+    def on_set_dstports(self, event, name, newvalue, oldvalue=None):
+        # pylint: disable=unused-argument
         if self.proto not in ('tcp', 'udp'):
             raise ValueError(
                 'dstports valid only for \'tcp\' and \'udp\' protocols')
 
     # noinspection PyUnusedLocal
     @qubes.events.handler('property-pre-set:icmptype')
-    def on_set_icmptype(self, _event, _prop, _new_value, _old_value=None):
+    def on_set_icmptype(self, event, name, newvalue, oldvalue=None):
+        # pylint: disable=unused-argument
         if self.proto not in ('icmp',):
             raise ValueError('icmptype valid only for \'icmp\' protocol')
 
     # noinspection PyUnusedLocal
     @qubes.events.handler('property-set:proto')
-    def on_set_proto(self, _event, _prop, new_value, _old_value=None):
+    def on_set_proto(self, event, name, newvalue, oldvalue=None):
-        if new_value not in ('tcp', 'udp'):
+        # pylint: disable=unused-argument
+        if newvalue not in ('tcp', 'udp'):
             self.dstports = qubes.property.DEFAULT
-        if new_value not in ('icmp',):
+        if newvalue not in ('icmp',):
             self.icmptype = qubes.property.DEFAULT
 
     @qubes.events.handler('property-del:proto')
-    def on_del_proto(self, _event, _prop, _old_value):
+    def on_del_proto(self, event, name, oldvalue):
+        # pylint: disable=unused-argument
         self.dstports = qubes.property.DEFAULT
         self.icmptype = qubes.property.DEFAULT
 

qubes/mgmt.py

@@ -110,13 +110,13 @@ class QubesMgmt(object):
     #
 
     @not_in_api
-    def fire_event_for_permission(self, *args, **kwargs):
+    def fire_event_for_permission(self, **kwargs):
         return self.src.fire_event_pre('mgmt-permission:{}'.format(self.method),
-            self.dest, self.arg, *args, **kwargs)
+            self.dest, self.arg, **kwargs)
 
     @not_in_api
-    def fire_event_for_filter(self, iterable, *args, **kwargs):
+    def fire_event_for_filter(self, iterable, **kwargs):
-        for selector in self.fire_event_for_permission(*args, **kwargs):
+        for selector in self.fire_event_for_permission(**kwargs):
             iterable = filter(selector, iterable)
         return iterable
 

qubes/qmemman/__init__.py

@@ -142,8 +142,8 @@ class SystemState(object):
                 if dom_name is not None:
                     try:
                         qubes.Qubes().domains[str(dom_name)].fire_event(
-                            'status:no-error', 'no-error',
+                            'status:no-error', status='no-error',
-                            slow_memset_react_msg)
+                            msg=slow_memset_react_msg)
                     except LookupError:
                         pass
                 self.domdict[i].slow_memset_react = False
@@ -154,8 +154,8 @@ class SystemState(object):
                 if dom_name is not None:
                     try:
                         qubes.Qubes().domains[str(dom_name)].fire_event(
-                            'status:no-error', 'no-error',
+                            'status:no-error', status='no-error',
-                            no_progress_msg)
+                            msg=no_progress_msg)
                     except LookupError:
                         pass
                 self.domdict[i].no_progress = False
@@ -345,8 +345,8 @@ class SystemState(object):
                                     try:
                                         qubes.Qubes().domains[str(
                                             dom_name)].fire_event(
-                                            'status:error', 'error',
+                                            'status:error', status='error',
-                                            no_progress_msg)
+                                            msg=no_progress_msg)
                                     except LookupError:
                                         pass
                             else:
@@ -361,8 +361,8 @@ class SystemState(object):
                                     try:
                                         qubes.Qubes().domains[str(
                                             dom_name)].fire_event(
-                                            'status:error', 'error',
+                                            'status:error', status='error',
-                                            slow_memset_react_msg)
+                                            msg=slow_memset_react_msg)
                                     except LookupError:
                                         pass
                     self.mem_set(dom, self.get_free_xen_memory() + self.domdict[dom].memory_actual - self.XEN_FREE_MEM_LEFT)

qubes/tests/__init__.py

@@ -130,7 +130,7 @@ class TestEmitter(qubes.events.Emitter):
     >>> emitter = TestEmitter()
     >>> emitter.fired_events
     Counter()
-    >>> emitter.fire_event('event', 1, 2, 3, spam='eggs', foo='bar')
+    >>> emitter.fire_event('event', spam='eggs', foo='bar')
     >>> emitter.fired_events
     Counter({('event', (1, 2, 3), (('foo', 'bar'), ('spam', 'eggs'))): 1})
     '''
@@ -141,15 +141,14 @@ class TestEmitter(qubes.events.Emitter):
         #: :py:class:`collections.Counter` instance
         self.fired_events = collections.Counter()
 
-    def fire_event(self, event, *args, **kwargs):
+    def fire_event(self, event, **kwargs):
-        effects = super(TestEmitter, self).fire_event(event, *args, **kwargs)
+        effects = super(TestEmitter, self).fire_event(event, **kwargs)
-        self.fired_events[(event, args, tuple(sorted(kwargs.items())))] += 1
+        self.fired_events[(event, tuple(kwargs.items()))] += 1
         return effects
 
-    def fire_event_pre(self, event, *args, **kwargs):
+    def fire_event_pre(self, event, **kwargs):
-        effects = super(TestEmitter, self).fire_event_pre(event, *args,
+        effects = super(TestEmitter, self).fire_event_pre(event, **kwargs)
-            **kwargs)
+        self.fired_events[(event, tuple(kwargs.items()))] += 1
-        self.fired_events[(event, args, tuple(sorted(kwargs.items())))] += 1
         return effects
 
 def expectedFailureIfTemplate(templates):
@@ -349,53 +348,57 @@ class QubesTestCase(unittest.TestCase):
                     dev_class, (": " + msg) if msg else "")
             )
 
-    def assertEventFired(self, emitter, event, args=None, kwargs=None):
+    def assertEventFired(self, subject, event, kwargs=None):
         '''Check whether event was fired on given emitter and fail if it did
         not.
 
-        :param emitter: emitter which is being checked
+        :param subject: emitter which is being checked
         :type emitter: :py:class:`TestEmitter`
         :param str event: event identifier
-        :param list args: when given, all items must appear in args passed to \
-            an event
         :param list kwargs: when given, all items must appear in kwargs passed \
             to an event
         '''
 
-        for ev, ev_args, ev_kwargs in emitter.fired_events:
+        will_not_match = object()
+        for ev, ev_kwargs in subject.fired_events:
             if ev != event:
                 continue
-            if args is not None and any(i not in ev_args for i in args):
+            if kwargs is not None:
-                continue
+                ev_kwargs = dict(ev_kwargs)
-            if kwargs is not None and any(i not in ev_kwargs for i in kwargs):
+                if any(ev_kwargs.get(k, will_not_match) != v
+                        for k, v in kwargs.items()):
                     continue
 
             return
 
-        self.fail('event {!r} did not fire on {!r}'.format(event, emitter))
+        self.fail('event {!r} {}did not fire on {!r}'.format(
+            event, ('' if kwargs is None else '{!r} '.format(kwargs)), subject))
 
 
-    def assertEventNotFired(self, emitter, event, args=None, kwargs=None):
+    def assertEventNotFired(self, subject, event, kwargs=None):
         '''Check whether event was fired on given emitter. Fail if it did.
 
-        :param emitter: emitter which is being checked
+        :param subject: emitter which is being checked
         :type emitter: :py:class:`TestEmitter`
         :param str event: event identifier
-        :param list args: when given, all items must appear in args passed to \
-            an event
         :param list kwargs: when given, all items must appear in kwargs passed \
             to an event
         '''
 
-        for ev, ev_args, ev_kwargs in emitter.fired_events:
+        will_not_match = object()
+        for ev, ev_kwargs in subject.fired_events:
             if ev != event:
                 continue
-            if args is not None and any(i not in ev_args for i in args):
+            if kwargs is not None:
-                continue
+                ev_kwargs = dict(ev_kwargs)
-            if kwargs is not None and any(i not in ev_kwargs for i in kwargs):
+                if any(ev_kwargs.get(k, will_not_match) != v
+                        for k, v in kwargs.items()):
                     continue
 
-            self.fail('event {!r} did fire on {!r}'.format(event, emitter))
+            self.fail('event {!r} {}did fire on {!r}'.format(
+                event,
+                ('' if kwargs is None else '{!r} '.format(kwargs)),
+                subject))
 
         return
 

qubes/tests/app.py

@@ -67,7 +67,8 @@ class TC_30_VMCollection(qubes.tests.QubesTestCase):
         self.vms.add(self.testvm1)
         self.assertIn(1, self.vms)
 
-        self.assertEventFired(self.app, 'domain-add', args=[self.testvm1])
+        self.assertEventFired(self.app, 'domain-add',
+            kwargs={'vm': self.testvm1})
 
         with self.assertRaises(TypeError):
             self.vms.add(object())
@@ -122,7 +123,8 @@ class TC_30_VMCollection(qubes.tests.QubesTestCase):
         del self.vms['testvm2']
 
         self.assertCountEqual(self.vms.vms(), [self.testvm1])
-        self.assertEventFired(self.app, 'domain-delete', args=[self.testvm2])
+        self.assertEventFired(self.app, 'domain-delete',
+            kwargs={'vm': self.testvm2})
 
     def test_100_get_new_unused_qid(self):
         self.vms.add(self.testvm1)

qubes/tests/init.py

@@ -342,7 +342,8 @@ class TC_30_VMCollection(qubes.tests.QubesTestCase):
         self.vms.add(self.testvm1)
         self.assertIn(1, self.vms)
 
-        self.assertEventFired(self.app, 'domain-add', args=[self.testvm1])
+        self.assertEventFired(self.app, 'domain-add',
+            kwargs={'vm': self.testvm1})
 
         with self.assertRaises(TypeError):
             self.vms.add(object())
@@ -395,7 +396,8 @@ class TC_30_VMCollection(qubes.tests.QubesTestCase):
         del self.vms['testvm2']
 
         self.assertCountEqual(self.vms.vms(), [self.testvm1])
-        self.assertEventFired(self.app, 'domain-delete', args=[self.testvm2])
+        self.assertEventFired(self.app, 'domain-delete',
+            kwargs={'vm': self.testvm2})
 
     def test_100_get_new_unused_qid(self):
         self.vms.add(self.testvm1)

qubes/tests/tools/qvm_device.py

@@ -113,9 +113,9 @@ class TC_00_Actions(qubes.tests.QubesTestCase):
         )
         qubes.tools.qvm_device.attach_device(args)
         self.assertEventFired(self.vm1,
-            'device-attach:testclass', [self.device])
+            'device-attach:testclass', kwargs={'device': self.device})
         self.assertEventNotFired(self.vm2,
-            'device-attach:testclass', [self.device])
+            'device-attach:testclass', kwargs={'device': self.device})
 
     def test_011_double_attach(self):
         args = TestNamespace(

qubes/tools/qubes_monitor_layout_notify.py

@@ -53,14 +53,14 @@ def main(args=None):
 
     subprocess.check_call(['killall', '-HUP', 'qubes-guid'])
     if args.vm:
-        args.vm.fire_event('monitor-layout-change', monitor_layout)
+        args.vm.fire_event('monitor-layout-change', layout=monitor_layout)
     else:
         threads = []
 
         for vm in args.app.domains:
             thread = threading.Thread(name=vm.name, target=vm.fire_event,
                 args=('monitor-layout-change',),
-                kwargs={'monitor_layout': monitor_layout})
+                kwargs={'layout': monitor_layout})
             threads.append(thread)
             thread.run()
 

qubes/vm/__init__.py

@@ -68,14 +68,14 @@ class Features(dict):
 
     def __delitem__(self, key):
         super(Features, self).__delitem__(key)
-        self.vm.fire_event('domain-feature-delete', key)
+        self.vm.fire_event('domain-feature-delete', key=key)
 
     def __setitem__(self, key, value):
         if value is None or isinstance(value, bool):
             value = '1' if value else ''
         else:
             value = str(value)
-        self.vm.fire_event('domain-feature-set', key, value)
+        self.vm.fire_event('domain-feature-set', key=key, value=value)
         super(Features, self).__setitem__(key, value)
 
     def clear(self):

qubes/vm/mix/net.py

@@ -337,7 +337,8 @@ class NetVMMixin(qubes.events.Emitter):
         new_netvm = self.netvm
         if new_netvm == old_netvm:
             return
-        self.fire_event('property-set:netvm', 'netvm', new_netvm, old_netvm)
+        self.fire_event('property-set:netvm',
+            name='netvm', newvalue=new_netvm, oldvalue=old_netvm)
 
     @qubes.events.handler('property-pre-set:netvm')
     def on_property_pre_set_netvm(self, event, name, new_netvm, old_netvm=None):
@@ -378,7 +379,7 @@ class NetVMMixin(qubes.events.Emitter):
             self.create_qdb_entries()
             self.attach_network()
 
-            new_netvm.fire_event('net-domain-connect', self)
+            new_netvm.fire_event('net-domain-connect', vm=self)
 
     @qubes.events.handler('net-domain-connect')
     def on_net_domain_connect(self, event, vm):

qubes/vm/qubesvm.py

@@ -1287,7 +1287,7 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
                 shutil.copy(src.icon_path, self.icon_path)
 
         # fire hooks
-        self.fire_event('domain-clone-files', src)
+        self.fire_event('domain-clone-files', src=src)
 
     #
     # methods for querying domain state