After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 656330 - Pythonic support for GDBus server objects
Pythonic support for GDBus server objects
Status: RESOLVED OBSOLETE
Product: pygobject
Classification: Bindings
Component: introspection
Git master
Other Linux
: Low enhancement
: ---
Assigned To: martin.pitt
Python bindings maintainers
Depends on: 656325
Blocks: 705069
 
 
Reported: 2011-08-11 10:25 UTC by Martin Pitt
Modified: 2018-01-10 20:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
sketch how this could look like (7.91 KB, text/plain)
2011-09-01 08:44 UTC, Martin Pitt
Details
"Fixed" version of Martin's sketch that uses the new register_object_with_closure API (7.61 KB, text/x-python)
2016-03-25 13:39 UTC, Thomas Liebetraut
Details
WIP: Proposal for DBus overrides (15.79 KB, text/x-python)
2016-04-09 11:12 UTC, Thomas Liebetraut
Details

Description Martin Pitt 2011-08-11 10:25:32 UTC
Client-side GDBus has worked well since the last pygi sprint in January, now it's time to make servers (i. e. exporting GDBus objects to the bus) work as well.

I already fixed some minor missing things and crashes for this: pygobject commits 18a240cc4 and 5189b360c, and glib commits 04969571b, 21c22914, c783bfd4e7, b2c6b801bc.

A bigger dependency is to make the GDBusInterfaceVTable struct introspection friendly/bindable, I sent a patch for this to bug 656325.

With these, this test script now works fine:
  http://people.canonical.com/~pitti/tmp/gdbus-server.py
This registers a simple object on the session bus and does a couple of gdbus calls to verify that method calling (success and failure) and property getting/setting works.

What's left is to create overrides which provides a pythonic and friendly wrapping around this. In Python we don't need to ask the programmer to do mundane task like creating an introspection XML and writing these "big switch" method_call()/get_property() functions.

I'd like to bring back the decorators from python-dbus:

   http://dbus.freedesktop.org/doc/dbus-python/doc/tutorial.html#exporting-objects

i. e. with decorators specifying the interface names and in/out signature, plus optional annotations we can figure out the introspection XML and provide generic method_call()/get_property()/set_property() handlers which will automatically dispatch incoming requests to exported objects.
Comment 1 Martin Pitt 2011-09-01 08:44:11 UTC
Created attachment 195366 [details]
sketch how this could look like

This is a first sketch how this could look like. I worked on this during the recent hackfest. It actually works if you apply the rejected glib patch in bug 656325. The _Gio_DBusMethodInfo, Gio_bus_method, and GioDBusServiceObject classes are meant to eventually go into the pygobject Gio overrides, and it's not a whole lot of code (at that point they will of course be renamed to Gio.*). The second part of the script then exports a demo object "PiwareDemo", which looks pretty much like with dbus-python.

However, I stopped working on this, as we need to resolve bug 656554 first to make any progress.

So I'm mainly attaching this to avoid losing the demo script. :)
Comment 2 Martin Pitt 2012-04-23 06:32:12 UTC
Still blocked on bug 656554 and bug 656325. However, in the meantime there is python3-dbus, so you now can use dbus for python3 projects just fine, and this is not urgent any more.
Comment 3 Andy Grover 2014-12-08 21:21:44 UTC
(In reply to comment #2)
> Still blocked on bug 656554 and bug 656325. However, in the meantime there is
> python3-dbus, so you now can use dbus for python3 projects just fine, and this
> is not urgent any more.

No, it's still urgent, because although python-dbus is still maintained, it is not developed any further. To any missing feature the reply is "use gdbus" so we really do need to get Python GDBus server objects working. Any idea how much work we'd be looking at?
Comment 4 Simon Feltman 2014-12-09 03:01:46 UTC
There is a patch sitting around in bug 656325 which might actually get this working. Seems that it needs review, I'm sure testing would also be appreciated.
Comment 5 Nils Philippsen 2015-09-02 14:22:08 UTC
Bug #656325 is resolved now, so is this one here still a thing, or is everything in place?
Comment 6 Thomas Liebetraut 2016-03-25 13:39:55 UTC
Created attachment 324753 [details]
"Fixed" version of Martin's sketch that uses the new register_object_with_closure API

I can confirm that this bug is still a thing. The resolution in bug 656325 allows having server objects from PyGI in the first place but the resulting API is far from being "pythonic".
I can also confirm that Martin's sketch from comment 1 works (in stable) with minimal changes as the API everyone agreed on in bug 656325 differs from the original proposal.

Is anyone still working on this? I'd be willing to spend some time on this, but I'd probably need some mentoring to meet the design criteria for pygobject overrides.
Comment 7 Simon Feltman 2016-03-26 00:46:26 UTC
(In reply to Thomas Liebetraut from comment #6)
> Is anyone still working on this? I'd be willing to spend some time on this,
> but I'd probably need some mentoring to meet the design criteria for
> pygobject overrides.

There is an external project which looks like it might be filling this gap:
https://github.com/LEW21/pydbus

For override guidelines, I would read through the following document:
https://wiki.gnome.org/Projects/PyGObject/OverrideGuidelines
Comment 8 Thomas Liebetraut 2016-03-28 23:16:58 UTC
(In reply to Simon Feltman from comment #7)
> There is an external project which looks like it might be filling this gap:
> https://github.com/LEW21/pydbus

I'm aware of this project, but unfortunately it does not support exporting dbus objects as far as I can see. See also: https://github.com/LEW21/pydbus/issues/7
Besides, I think it would be nice if PYGObject had a nice interface to GDBusObject(Skeleton) just as it has to GDBusProxy.


> For override guidelines, I would read through the following document:
> https://wiki.gnome.org/Projects/PyGObject/OverrideGuidelines

I'm also aware of this document (thanks anyway :-)) but I struggle with some details, especially the first and last points.

More specifically:
Martin Pitt's initial code uses an own Python-only implementation of DBusMethodInfo and creates the XML manually. The original DBusMethodInfo from Gio is available in PyGI and allows generating the introspection XML using (ultimately) g_dbus_interface_generate_xml() (which is also available in PyGI).
In C, this works perfectly fine: define some values and build up a GDBusInterfaceInfo struct and generate the introspection XML from it. The only thing that stops PyGI from using the same code is that zero-terminated arrays like the in_args member of GDBusMethodInfo [1] cannot be set using gobject-introspection. Reading is fine, though.

I spent yesterday's afternoon digging into pygobject and gobject-introspection and finding out whether this can changed, but it seems that this is far more complicated. gi's g_field_info_set_field() refuses to work on any pointer type. pygobject adds and exception for class and interface members, but doing the same for zero-terminated C-arrays requires a kind memory handling that gi does not support.

According to the Override Guidelines, it should be better to change the underlying API to make it more introspection-friendly, e.g. by using GLists in the introspection data structures.
In order to get things done, Martin Pitt's approach of re-implementing the XML-generation is more reasonable.
Gjs does neither and requires introspection XML to be explicitly provided by the developer.

As an alternative, is it possible, in pygobject, to have C-level overrides? I.e. provide classes implemented in C that override the GDBus*Info classes but have a proper corresponding GDBus struct. Then handle all the member access inside pygobject and update the GDBus structs without gobject-introspection. This way, we could still use GDBus' XML infrastructure. I think this somewhat matches the "prefer adapter patterns" but is not really an adapter from the Python point of view. Is there already an example of this somewhere in pygobject that I can look at? Does this even make sense at all?


[1] https://developer.gnome.org/gio/stable/gio-D-Bus-Introspection-Data.html#GDBusMethodInfo-struct
Comment 9 Thomas Liebetraut 2016-04-09 11:12:34 UTC
Created attachment 325628 [details]
WIP: Proposal for DBus overrides

Attached is my current WIP based on Martin's original sketch.

I did not follow my initial idea of overriding the GDBus Annotation data structures in C because I am under the impression that only the core GObject stuff in pygobject seems to be done in C. Gio seems too remote from GObject to receive that status, IMHO. Instead I mimicked the necessary parts in Python, including the XML generation and overall structure. If one day we have proper introspection support for the GDBus introspection stuff, it should be easy to migrate.
The rest of my API is based on a mixture of python-dbus and the existing pygobject overrides.

My dbus_property implementation can be used on its own (decorating getters and setters) but it also allows decorating Python properties and even GObject properties to better integrate with existing classes that are to be exposed via DBus. The GObject integration is functionally complete but it does not register with the owner's __gproperties__ member, yet (requires changes in _propertyhelper.py). This is a thing on my todo list.

Other items on that list:
* DBus annotations
* Support necessary methods from GDBusObjectSkeleton and GDBusObject
* Python3 compatibility
* Python3 type hint support
* Signals
* Maybe implement the Properties interface directly. The closures do not allow nice error propagation.


Re signals: I am still not sure how to do this properly. dbus-python automatically emits the signal as soon as a decorated method is called. GObject.Signal is intended as separate class attribute, it does not emit the signal if a decorated method is called. I'm leaning towards explicit emit calls (like GObject.Signal) but I'm not sure if decorating is useful in this case.

I'm grateful for any comments and suggestions, especially if the API is okay like it is.
Comment 10 Simon Feltman 2016-04-19 07:34:03 UTC
(In reply to Thomas Liebetraut from comment #8)
> According to the Override Guidelines, it should be better to change the
> underlying API to make it more introspection-friendly, e.g. by using GLists
> in the introspection data structures.
> In order to get things done, Martin Pitt's approach of re-implementing the
> XML-generation is more reasonable.
> Gjs does neither and requires introspection XML to be explicitly provided by
> the developer.

Since the proposed API looks higher level and the XML generation vs. defining structures is just an implementation detail, I think using XML generation is fine.

I hope to have time to look at the rest of this later in the week...
Comment 11 Simon Feltman 2016-04-25 05:14:17 UTC
(In reply to Thomas Liebetraut from comment #9)
> Created attachment 325628 [details]
> WIP: Proposal for DBus overrides

I think overall this is looks good. There are some major integration questions and a few nitpicks.

* Where should this module go? should it become part of the Gio overrides or should we keep it as a separate module like gi.dbus.service? It might be a good idea to ping Simon McVittie since he seems to be keeping dbus-python up to date...
* It would be good to have a noted list of differences with dbus-python. 
* We need lots of tests :)

Nit picks:
* Needs Python 3 compatibility as you noted. I saw a few "raise" calls which were old style.
* Don't use dunder-dunder names: "__dbusinfo__". Even though this will likely never clash with core stuff, it sets a precedent for future contributors who will follow the style. For example we have "__info__" on a lot of things in gi which was an awful idea.
Comment 12 Thomas Liebetraut 2016-04-27 20:46:30 UTC
Thanks for your review and your comments.

(In reply to Simon Feltman from comment #11)
> I think overall this is looks good. There are some major integration
> questions and a few nitpicks.
> 
> * Where should this module go? should it become part of the Gio overrides or
> should we keep it as a separate module like gi.dbus.service?

I was thinking it should go to the Gio overrides. It's really just a binding-specific (pythonic) version of the Gio classes. We could argue about the decorators. I'd probably put them in the DBusServiceObject class so that we could use "@DBusSerbiceObject.{method,property}" instead of separate (and unrelated) names for the decorators.
I don't really like the gi.dbus.service approach because dbus support is a core feature of Gio and should therefore be available via "from gi.repository import Gio".


> It might be a
> good idea to ping Simon McVittie since he seems to be keeping dbus-python up
> to date...

Will do.


> * It would be good to have a noted list of differences with dbus-python.

Documentation was implicit on my todo list as the current version lacks it completely ;-)
Should this go into the API/sphinx documentation or into a separate text file? If so, where? docs/ seems to be the wrong directory for that.


> Nit picks:
> * Don't use dunder-dunder names: "__dbusinfo__". Even though this will
> likely never clash with core stuff, it sets a precedent for future
> contributors who will follow the style. For example we have "__info__" on a
> lot of things in gi which was an awful idea.

Ok. Actually, I saw the "__info__" precedent and followed the style ;-)
I will move to "__dbusinfo" instead.
Comment 13 Simon McVittie 2016-04-28 12:13:31 UTC
(In reply to Simon Feltman from comment #11)
> It might be a
> good idea to ping Simon McVittie since he seems to be keeping dbus-python up
> to date...

I'll try to review this sometime soon.

dbus-python is unmaintained, and I am its unmaintainer. I would love to be able to tell people to stop using it. (Actually, I already do that, but it would be nice to have a definite recommendation to point them to.)

The major thing I would like to push is that it should be Pythonic in the sense of "in the face of ambiguity, resist the temptation to guess". If you are calling or implementing a D-Bus API, you should *already* know what its signature is - if you don't, you can't ever hope to be interoperable - so it should never be necessary to introspect the service to find out how to interpret the arguments. I can't fix this in dbus-python because it would break its biggest feature (being compatible with older dbus-python), but new libraries absolutely should - maybe you already did.

Another design flaw in dbus-python is that its decorators are rather magical, and in particular they make it impossible to implement two interfaces that both have a DoSomething() method. That is probably fixable, but so far I haven't dared to try.

> > Nit picks:
> > * Don't use dunder-dunder names: "__dbusinfo__". Even though this will
> > likely never clash with core stuff, it sets a precedent for future
> > contributors who will follow the style.
>
> I will move to "__dbusinfo" instead.

I'm not sure this is actually appropriate.

In Python, names with __underscores_before_and_after__ are for when you are providing a magic "hook" that can be included in *someone else's namespace*. That's how the core Python developers use __str__ and __int__ and so on. I think it would be OK to use GLib-specific hooks *if they have a prefix* (to avoid colliding with Python core or with other libraries), for example __gproperties__ and maybe __g_dbus_info__ or __glib_dbus_info__. I think I have a __dbus_something__ in dbus-python already, possibly in exception handling?

Names with __double_underscores_before are like C++ "private": behind the scenes, they're mangled into _TheClassName__double_underscores_before to avoid collisions.

Names with _single_underscore_before are not used 100% consistently, but they're usually like C++ "protected".
Comment 14 Simon McVittie 2016-04-28 12:15:16 UTC
I'll find it a lot easier to review the design (hence more likely to happen) if there are examples, tests and/or documentation that I can look at.
Comment 15 Patrick Griffis (tingping) 2016-07-09 00:34:23 UTC
Comment on attachment 325628 [details]
WIP: Proposal for DBus overrides

(In reply to Thomas Liebetraut)

> from IPython.external.path._path import path

Can be removed?

> in_signature_list = GLib.Variant.split_signature(in_signature)

Doesn't handle empty signature:

    in_signature_list = GLib.Variant.split_signature(in_signature) if in_signature else []

> raise TypeError, '...'

Needs Python 3 style. Same for print()'s

> invocation.return_value(GLib.Variant('(' + info.out_args[0].signature + ')', (ret,)))

Doesn't handle methods with no return value:

    if ret is None and not info.out_args[0].signature:
        return # No return value


In some basic testing it largely seems to work. The big missing thing is annotations for signals of course.
Comment 16 Patrick Griffis (tingping) 2016-07-09 22:13:46 UTC
So I took this and modified it quite a bit and rebased a dbus-python
project on top of it with good results:

https://github.com/pithos/pithos/commit/037fa10aaa9c66a096c75365a560b4c27351b6d9
https://github.com/pithos/pithos/blob/037fa10aaa9c66a096c75365a560b4c27351b6d9/pithos/plugins/dbus_helpers/_dbus_helper.py

Some modifications I made:

- Implemented dbus_signal
- Used ElementTree to make xml output
- Fixed various incorrect signature handling
- Fixed various typos
- Fixed on Python 3 (py2 untested now)
- Made connection and object-path construct gobject properties
- Various renamings

Remaining API concerns of mine:

Currently you need to own the bus name before you initialize
the object; This could possibly be cleaner or add multiple
constructors that handle this for you.

A clean way to make a setter for a property that is private and not
exposed over dbus?

I didn't touch dbus_property personally but it looks like it could use
some cleanup and isn't thoroughly tested in my mpris implementation.
Comment 17 Patrick Griffis (tingping) 2016-07-09 22:21:13 UTC
(In reply to Patrick Griffis (tingping) from comment #16)
> Currently you need to own the bus name before you initialize
> the object; This could possibly be cleaner or add multiple
> constructors that handle this for you.

Just wanted to clarify that was my modification as I didn't really like
the _add_to_connection() and _remove_from_connection() (which was broken also)
and wanted an alternative.
Comment 18 Simon McVittie 2016-07-19 09:26:06 UTC
Sorry, I have other things I need to prioritize and can't review this in detail at the moment.

(In reply to Martin Pitt from comment #0)
> I'd like to bring back the decorators from python-dbus

Some lessons learned from dbus-python and Telepathy:

* People will want to implement methods of the same name in
  different interfaces. I'd recommend dissociating the D-Bus method
  name from the Python method name, or at least making it possible.

* People will want to overload methods. Either make it possible,
  or have a clear explanation that it isn't. (Overloading is not part
  of the interoperable subset of D-Bus, IMO.)

* dbus-python has had bug reports asking for "varargs" methods that
  do not introspect the Python function object. That's probably a
  good idea here (I'm reluctant to introduce things like that in
  dbus-python because I don't want to compromise its biggest feature,
  namely 100% compatibility with dbus-python.)

* Methods that return asynchronously should be first-class citizens.
  In dbus-python, they're somewhat awkward (and in dbus-glib, they
  needed special annotations).

(In reply to Patrick Griffis (tingping) from comment #16)
> Currently you need to own the bus name before you initialize
> the object; This could possibly be cleaner or add multiple
> constructors that handle this for you.

In GDBus, you will need to be able to export (register) objects *before* claiming your bus name.

Background: as soon as you take your bus name, clients of your API will expect to be able to call your methods. In libdbus (dbus-python, dbus-glib), exporting objects after claiming the bus name was (strictly speaking) wrong, but in practice you'd usually get away with it, as long as you exported the objects within the same main-loop iteration as being told you had obtained your bus name.

In GDBus, that trick doesn't work, because the GDBusConnection is dispatched by a worker thread - so it will potentially already be responding "no, I don't know that object" before you have a chance to jump in and export the object.
Comment 19 Patrick Griffis (tingping) 2016-07-19 12:49:17 UTC
> In GDBus, you will need to be able to export (register) objects *before* claiming your bus name.

Upon more usage I was mistaken how it was behaving and my current usage of it now behaves as such. Thanks for the info.
Comment 20 GNOME Infrastructure Team 2018-01-10 20:10:58 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/pygobject/issues/19.