| Summary: | Python bindings should offer @dbus.service.property and automatically build proper introspection data | ||
|---|---|---|---|
| Product: | dbus | Reporter: | Zygmunt Krynicki <zygmunt.krynicki> |
| Component: | python | Assignee: | Simon McVittie <smcv> |
| Status: | RESOLVED DUPLICATE | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | medium | CC: | barry, marko.kohtala |
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: |
attachment-30358-0.html
attachment-30358-1.dat 0001-Add-property-decorator.patch 0002-Add-_reflect_on_property-to-InterfaceType-and-use-it.patch 0003-Add-implementation-of-Get-GetAll-Set-to-dbus.service.patch |
||
Some quick feedback: (In reply to comment #0) > @dbus.service.property( > dbus_signature="s", dbus_interface="org.example.Thing") In new code, please don't repeat the design mistake of requiring the name of the D-Bus method/signal to match the name of the Python method. (The name of the Python method can be a default for the D-Bus method, if you want.) In new code, please don't repeat the design mistake of the signature being optional (although it probably isn't, because if it was, I can't see how Introspect() would work). It should be mandatory. > @prop2.setter > def prop2(self, value): > self._prop2 = value Does that actually work, syntactically? I think I'd prefer a less decorator-ful approach, maybe with a magic class attribute like GObject properties have: class MyThing: def get_readable_property(self): return 'bees' def set_rw_property(self, value): if not value: raise TypeError('ReadableProperty must be non-empty') self.__rw = value # ... etc. __dbus_properties__ = { 'org.example.Thing': { 'ReadableProperty': dbus.service.Property('s', get_readable_property, None), 'ReadWriteProperty': dbus.service.Property('s', get_rw_property, set_rw_property), 'WritableProperty': dbus.service.Property('s', None, set_w_property), }, } (but I could be persuaded otherwise). Setters should be able to be asynchronous with a callback for success/failure, at least optionally (perhaps making it mandatory would be simpler). In telepathy-glib they can't, and with hindsight, that was a mistake. Should getters be able to be asynchronous? Probably not... Helper code for emitting PropertiesChanged on property changes from the Python side would be nice. (In reply to comment #1) > __dbus_properties__ = { > 'org.example.Thing': { > 'ReadableProperty': dbus.service.Property('s', > get_readable_property, None), > 'ReadWriteProperty': dbus.service.Property('s', > get_rw_property, set_rw_property), > 'WritableProperty': dbus.service.Property('s', > None, set_w_property), > }, > } If you like that syntax, a million penguin points if you can make methods and signals work more like that too, with backwards compatibility for the old decorator-based approach (this would address Bug #25125 and Bug #31777). On Jun 18, 2013, at 03:25 PM, bugzilla-daemon@freedesktop.org wrote: >> @prop2.setter >> def prop2(self, value): >> self._prop2 = value > >Does that actually work, syntactically? Yes, and in fact, it's the preferred way to add properties that allow setting and deleting, e.g. class Foo: def __init__(self, a): self._a = a @property def a(self): return self._a @a.setter def a(self, value): self._a = value @a.deleter def a(self): self._a = None foo = Foo(1) print(foo.a) foo.a = 7 print(foo.a) del foo.a print(foo.a) >I think I'd prefer a less decorator-ful approach, maybe with a magic class >attribute like GObject properties have: > >class MyThing: > def get_readable_property(self): > return 'bees' > > def set_rw_property(self, value): > if not value: > raise TypeError('ReadableProperty must be non-empty') > self.__rw = value > > # ... etc. > > __dbus_properties__ = { > 'org.example.Thing': { > 'ReadableProperty': dbus.service.Property('s', > get_readable_property, None), > 'ReadWriteProperty': dbus.service.Property('s', > get_rw_property, set_rw_property), > 'WritableProperty': dbus.service.Property('s', > None, set_w_property), > }, > } > >(but I could be persuaded otherwise). Seems rather verbose. I'm not a big fan of magic attributes because it's not obvious for the reader to know how those interact with the system. Besides, decorators rule! :) (In reply to comment #1) > Some quick feedback: > > (In reply to comment #0) > > @dbus.service.property( > > dbus_signature="s", dbus_interface="org.example.Thing") > > In new code, please don't repeat the design mistake of requiring the name of > the D-Bus method/signal to match the name of the Python method. (The name of > the Python method can be a default for the D-Bus method, if you want.) > > In new code, please don't repeat the design mistake of the signature being > optional (although it probably isn't, because if it was, I can't see how > Introspect() would work). It should be mandatory. > > > @prop2.setter > > def prop2(self, value): > > self._prop2 = value > > Does that actually work, syntactically? Yes, it does. It also matches the (relatively unknown) feature of python properties that have the same ability. > I think I'd prefer a less decorator-ful approach, maybe with a magic class > attribute like GObject properties have: > > class MyThing: > def get_readable_property(self): > return 'bees' > > def set_rw_property(self, value): > if not value: > raise TypeError('ReadableProperty must be non-empty') > self.__rw = value > > # ... etc. > > __dbus_properties__ = { > 'org.example.Thing': { > 'ReadableProperty': dbus.service.Property('s', > get_readable_property, None), > 'ReadWriteProperty': dbus.service.Property('s', > get_rw_property, set_rw_property), > 'WritableProperty': dbus.service.Property('s', > None, set_w_property), > }, > } > > (but I could be persuaded otherwise). I think that's orthogonal. I personally prefer decorators as they both share similarities to the python world and are easier to implement with subclassing. (But I'm also okay to be persuaded otherwise) > Setters should be able to be asynchronous with a callback for > success/failure, at least optionally (perhaps making it mandatory would be > simpler). In telepathy-glib they can't, and with hindsight, that was a > mistake. I haven't done any async dbus coding yet. How does that change the implementation of Set()? > Should getters be able to be asynchronous? Probably not... I'm fine with making them (this is a library). I just need to give it a try. > Helper code for emitting PropertiesChanged on property changes from the > Python side would be nice. Yeah. I was looking at that (my application didn't really need it but since I added the signal handler I though it'd be very easy to implement anyway). For read-write properties it would fire when value is different, for write only, I guess always? Thanks ZK (In reply to comment #3) > Yes, and in fact, it's the preferred way to add properties that allow setting > and deleting, e.g. > > class Foo: > def __init__(self, a): > self._a = a > @property > def a(self): > return self._a > @a.setter > def a(self, value): > self._a = value > @a.deleter > def a(self): > self._a = None Ah, I see, it's order-sensitive? (In reply to comment #4) > I personally prefer decorators as they both > share similarities to the python world and are easier to implement with > subclassing. (But I'm also okay to be persuaded otherwise) If you can implement them in a way that doesn't fall into the same traps as the signal and method decorators, fine. > > Setters should be able to be asynchronous with a callback for > > success/failure, at least optionally (perhaps making it mandatory would be > > simpler). In telepathy-glib they can't, and with hindsight, that was a > > mistake. > > I haven't done any async dbus coding yet. How does that change the > implementation of Set()? Instead of returning on success and raising an exception on failure, the Set() implementation would have to call a callback on success, or a different callback (with an exception as a parameter) on failure. That means it can do things like "fire off an async request to a network service and go back to the main loop, then when the response comes back, call the appropriate callback". > > Should getters be able to be asynchronous? Probably not... > > I'm fine with making them (this is a library). I just need to give it a try. Allowing asynchronous getters would make GetAll() really awkward to implement, so unless you can see a sensible reason, I would say "no" :-) Another question to ask early in the design is what can fail. In telepathy-glib, we don't allow Get() and GetAll() to fail, but we do allow Set() to fail. In Python, because absolutely anything can raise an exception, you might want to say that if your interface is this pseudocode: property Good: 's' get return 'yes' property Bad: 's' get raise MyError('no!') then you get: Get(iface, 'Good') -> success, 'yes' Get(iface, 'Bad') -> error, com.example.Errors.MyError 'no!' GetAll(iface) -> success, { 'Good': 'yes' } # 'Bad' is just omitted > For read-write properties it would fire when value is different, for > write only, I guess always? Well, the real point is to emit PropertiesChanged if something other than a D-Bus method call causes the change. For instance, imagine you're writing NetworkManager, you have a WirelessConnection object, and the bit-rate changes from 54Mbit to 11Mbit. You want an easy way to emit PropertiesChanged('blah.blah.blah.WirelessConnection', 'BitRate', '11000000') or something. Even when you're receiving a Set() call from D-Bus, it's not necessarily true that the new value is what the user wanted. Imagine you have a property with some sort of concept of normalization - a domain name that is always lower-case, perhaps. If the user said Set('...', 'DomainName', 'ExAmPlE.ORG'), you need a way to emit PropertiesChanged('...', 'DomainName', 'example.org') instead. (In reply to comment #5) > You want an easy way to emit > PropertiesChanged('blah.blah.blah.WirelessConnection', 'BitRate', > '11000000') or something. That's not actually how PropertiesChanged works - it's more like PropertiesChanged('...', {'BitRate': 11000000}, []) where the third argument is a list of the names of properties that have changed, but where the service does not want to tell you the new value (perhaps because it's very large or because it's expensive to compute). So you probably also need a way to group multiple property changes into one signal emission, and a way to flag properties as "this one is expensive, don't send it". If in doubt, I would recommend stealing API from designs that have been thought out well (e.g. TpDBusPropertiesMixin in telepathy-glib is pretty good[1], except for not allowing asynchronous 'set'; GDBusInterfaceSkeleton in GDBus is well-designed but rather GObject-centric, and not particularly obvious to use without wrapping it in generated code). [1] disclosure: I wrote a lot of it, so of course I'd say that. On Jun 18, 2013, at 04:14 PM, bugzilla-daemon@freedesktop.org wrote: >https://bugs.freedesktop.org/show_bug.cgi?id=65900 > >--- Comment #5 from Simon McVittie <simon.mcvittie@collabora.co.uk> --- >(In reply to comment #3) >> Yes, and in fact, it's the preferred way to add properties that allow setting >> and deleting, e.g. >> >> class Foo: >> def __init__(self, a): >> self._a = a >> @property >> def a(self): >> return self._a >> @a.setter >> def a(self, value): >> self._a = value >> @a.deleter >> def a(self): >> self._a = None > >Ah, I see, it's order-sensitive? Yes. There's really nothing magical here; the objects returned by the property decorators themselves have methods which can be used as decorators. They can actually be used in any order, and property objects have a .getter decorator, but that's rarely used since the getter is almost always defined first. http://docs.python.org/3/library/functions.html#property txdbus might also be a good place to steal ideas (or indeed you could use txdbus instead of dbus-python - I'm quite prepared to believe that it's better-designed). On Jun 18, 2013, at 04:31 PM, bugzilla-daemon@freedesktop.org wrote: >txdbus might also be a good place to steal ideas (or indeed you could use >txdbus instead of dbus-python - I'm quite prepared to believe that it's >better-designed). I didn't even know about it, thanks for the reference! I'll have to read up on it, but at first glance, maybe it would be cool to adapt this to Guido's tulip work and/or Python 3.3's "yield from" syntax. Created attachment 81021 [details] attachment-30358-0.html I have three patches with a direct work-in-progress port from python3 and my application code. I mainly want to get some feedback and see how this can start taking shape. I will post updates as I remove the duplicated _dbus_class_table-like feature from my code On Tue, Jun 18, 2013 at 6:35 PM, <bugzilla-daemon@freedesktop.org> wrote: > *Comment # 9 <https://bugs.freedesktop.org/show_bug.cgi?id=65900#c9> on bug > 65900 <https://bugs.freedesktop.org/show_bug.cgi?id=65900> from Barry > Warsaw <barry@python.org> * > > On Jun 18, 2013, at 04:31 PM, bugzilla-daemon@freedesktop.org wrote: > >txdbus might also be a good place to steal ideas (or indeed you could use > >txdbus instead of dbus-python - I'm quite prepared to believe that it's > >better-designed). > > > I didn't even know about it, thanks for the reference! I'll have to read up > on it, but at first glance, maybe it would be cool to adapt this to Guido's > tulip work and/or Python 3.3's "yield from" syntax. > > ------------------------------ > You are receiving this mail because: > > - You reported the bug. > > Created attachment 81022 [details]
attachment-30358-1.dat
Created attachment 81023 [details]
0001-Add-property-decorator.patch
Created attachment 81024 [details]
0002-Add-_reflect_on_property-to-InterfaceType-and-use-it.patch
Created attachment 81025 [details]
0003-Add-implementation-of-Get-GetAll-Set-to-dbus.service.patch
I've pushed that code (with minor updates) to https://github.com/zyga/dbus-python to the 'properties' branch. (I don't have a freedesktop account so I cannot use that) Thanks ZK This seems to be a duplicate of bug 26903. I had there a patch that did about the same thing. |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.
Hi. This is a feature request for being able to export properties from python without having to manually implement all the required infrastructure bits in each application. I actually wrote this code already and I'm looking for a way to upstream it. I've implemented this as a side-code to python3-dbus (as I need it today in my application) but the approach is generic enough, and close to upstream code enough to contribute back without major issues. For example, this is how one would create an object with three properties. The first property is read-only, the second one read-write and the last one is write-only. class Example(dbus.service.Object): @dbus.service.property( dbus_signature="s", dbus_interface="org.example.Thing") def prop1(self): return self._prop1 @dbus.service.property( dbus_signature="s", dbus_interface="org.example.Thing") def prop2(self): return self._prop2 @prop2.setter def prop2(self, value): self._prop2 = value @dbus.service.property( dbus_signature="s", dbus_interface="org.example.Thing") def prop2(self): return self._prop2 @dbus.service.property( dbus_signature="s", dbus_interface="org.example.Thing", setter=True) def prop3(self, value): return self._prop3 = value This class automatically implements Get(), Set() and GetAll(). The standard implementation of Intropsect() is improved to advertise all of the properties. The code that I have has no tests but I'd like to work on adding some (I'm still a bit new to python-dbus). I can easily post everything I have (it's a part of an open source application) but it would probably be a bit easier if I could work on top of the right git tree instead. I've got git://anongit.freedesktop.org/dbus/dbus-python and I'm looking around to see how to build/run it and add my code. Thanks Zygmunt Krynicki Thanks Zygmut