[In Progress] Port to TelepathyGLib

[tonadev]

Explanation

Continues with #389 but it's still in progress. This PR ports to TelepathyGLib.

Reason

The telepathy library does not have its bindings for Python 3, and porting Telepathy to its PyGObject binding is a pre requisite for the Port to Python 3 Project.

I tested it to see that activities continue working, and they do. However I encourage you to not trust me and test it by yourselves.

[quozl]

Thanks.

Some comments have been hit by your global search and replace; please review them again and consider whether they should change or not.

In terms of copyright, how does your patch relate to #389? Does it extend that work or supersede it?

Tested on Fedora 18 on OLPC OS 13.2.9 using Chat activity in collaboration. The Chat activity in which sharing is enabled appears to behave normally, but the other instances of Sugar do not show the shared activity. Logs from the first Chat activity have additional messages;

Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/dbus/connection.py", line 604, in msg_reply_handler
    reply_handler(*message.get_args_list(**get_args_opts))
  File "/usr/lib/python2.7/site-packages/sugar3/presence/activity.py", line 583, in __create_text_channel_cb
    ready_handler=self.__text_channel_ready_cb)
TypeError: GObject.__init__() takes exactly 0 arguments (2 given)
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/dbus/connection.py", line 604, in msg_reply_handler
    reply_handler(*message.get_args_list(**get_args_opts))
  File "/usr/lib/python2.7/site-packages/sugar3/presence/activity.py", line 587, in __create_tubes_channel_cb
    ready_handler=self.__tubes_channel_ready_cb)
TypeError: GObject.__init__() takes exactly 0 arguments (2 given)

Line numbers may be offset slightly, as I've done a git cherry-pick 283dde0 into my branch.

[tonadev]

Thanks.

Some comments have been hit by your global search and replace; please review them again and consider whether they should change or not.

@quozl Those comments referenced a telepathy object, and now they reference a TelepathyGLib object. If you disagree, tell me and I will revert the changes.

In terms of copyright, how does your patch relate to #389? Does it extend that work or supersede it?

@quozl I use the PR as a guide. Comparing the changes I did with the changes shown in that PR.

Tested on Fedora 18 on OLPC OS 13.2.9 using Chat activity in collaboration. The Chat activity in which sharing is enabled appears to behave normally, but the other instances of Sugar do not show the shared activity. Logs from the first Chat activity have additional messages;

Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/dbus/connection.py", line 604, in msg_reply_handler
    reply_handler(*message.get_args_list(**get_args_opts))
  File "/usr/lib/python2.7/site-packages/sugar3/presence/activity.py", line 583, in __create_text_channel_cb
    ready_handler=self.__text_channel_ready_cb)
TypeError: GObject.__init__() takes exactly 0 arguments (2 given)
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/dbus/connection.py", line 604, in msg_reply_handler
    reply_handler(*message.get_args_list(**get_args_opts))
  File "/usr/lib/python2.7/site-packages/sugar3/presence/activity.py", line 587, in __create_tubes_channel_cb
    ready_handler=self.__tubes_channel_ready_cb)
TypeError: GObject.__init__() takes exactly 0 arguments (2 given)

@quozl After the latest commit the error is not longer shown. Can you confirm it?

Line numbers may be offset slightly, as I've done a git cherry-pick 283dde0 into my branch.

[quozl]

Thanks. Tested with 3725c30.

Yes, the traceback is gone.

However, other instances of Sugar do not show the shared activity on neighbourhood view. Network packets for Clique are seen on both systems using tcpdump. There are no errors in shell.log on either system.

Not sure if it is related, but sometimes an activity fails to start with this traceback;

Traceback (most recent call last):  
  File "/usr/bin/sugar-activity", line 219, in <module>  
    main()  
  File "/usr/bin/sugar-activity", line 214, in main
    instance = create_activity_instance(activity_constructor, activity_handle)  
  File "/usr/bin/sugar-activity", line 48, in create_activity_instance  
    activity = constructor(handle)  
  File "/home/olpc/Activities/Chat.activity/activity.py", line 86, in __init__  
    super(Chat, self).__init__(handle)  
  File "/usr/lib/python2.7/site-packages/sugar3/activity/activity.py", line 430, in __init__  
    warn_if_none=False)  
  File "/usr/lib/python2.7/site-packages/sugar3/presence/presenceservice.py", line 81, in get_activity
    dbus_interface=CONN_INTERFACE_ACTIVITY_PROPERTIES)
  File "/usr/lib/python2.7/site-packages/dbus/proxies.py", line 70, in __call__
    return self._proxy_method(*args, **keywords)
  File "/usr/lib/python2.7/site-packages/dbus/proxies.py", line 145, in __call__
    **keywords)
  File "/usr/lib/python2.7/site-packages/dbus/connection.py", line 651, in call_blocking
    message, timeout)
dbus.exceptions.DBusException: org.freedesktop.DBus.Error.NoReply: Message did not receive a reply (timeout by message bus)

[tonadev]

@quozl @pro-panda @chimosky is the dbus library compatible with TelepathyGLib? Because when initializing a TelepathyGLib.Channel class it requires a connection object. So I used the 'global' variable _connection, however this variable is a dbus.ProxyObject class that TelepathyGlib.Channel doesn't accept, it requires a TelepathyGLib.Connection, and it requires a TelepathyGLib.DbusDaemon class. The dbus.ProxyObject is never used at least that you use it to extract the properties like the bus_name or the object_path. So, is it compatible or necessary? am I wrong with my observations? Is it a require needed to port to TelepathyGLib that I didn't knew?

[quozl]

I don't know. Best place to look would be the Telepathy source code. Second best place would be the Telepathy documentation, the PyGObject documentation, and other programs that use Telepathy. You might also look at the Chat activity and CollabWrapper which both call Channel.

[tonadev]

@quozl I took a fast look at chat and collabwrapper. Its seems like collabwrapper didnt worked with telepathy (and dbus) so they reverted the changes and chat doesnt use dbus (At least I didnt see any dbus import)

[quozl]

Thanks for the update. Let me know how you get on with the Telepathy source code. There's also a PyGObject API for DBus and DBusGLib, so perhaps that needs porting to as well. It may help to write a small program that uses the features of these APIs, and do some heavy debugging and tracing.

[tonadev]

@quozl I am not sure that the error comes from the use of TelepathyGLib with DBus.
Other thing that can be causing the error :

• I didn't found how to replace this: https://github.com/sugarlabs/sugar-toolkit-gtk3/pull/401/files#diff-d2af8a4081064bcfaf2457e9286beefdR186, so I didn't change it. Making the _ClientHandler work incorrectly (probably).

[quozl]

Ah, I see. Well, in porting to PyGObject API for Telepathy, we can't expect to use the static binding any longer; can you finish porting that line?

[tonadev]

@quozl The problem is that I haven't found the substitute for telepathy.server.DBusProperty. I've looking here: https://lazka.github.io/pgi-docs/#TelepathyGLib-0.12. Any suggestion?

[quozl]

No suggestions on a substitute, I've never done it; but some suggestions for next steps if I had to do it;

• interactively explore the telepathy.server.DBusProperties class and objects created from the class, e.g. with help(), dir(),
• instrument the code to print details of the class, e.g. dir(DBusProperties),
• read the source code for the static binding to Telepathy, paying attention to the definition of the class DBusProperties,
• read the source code for the PyGObject binding to Telepathy, looking for a class with a similar semantic basis,
• read other source code containing classes that implement a HandleChannels method,
• understand why the DBusProperties class is used as a parent of the _ClientHandler class.

[tonadev]

@chimosky which line? Here? or here? PS: TelepathyGLib.Channel.new() is deprecated, it was replaced by TelepathyGLib.SimpleClientFactory.ensure_channel() but it's more like the TelepathyGLib.Channel.new_from_properties(). And it requires a TelepathyGLib.Connection but the variable _connection is of type dbus.ProxyObject. See above

[chimosky]

The second one but it's okay now I see the method is deprecated.

[quozl]

Any update? Python 3 support landed for v0.113 as aa8a5e7, so a TelepathyGLib port is needed now.

[rhl-bthr]

Sorry, no.

I haven't had the time to look into it

[tonadev]

I am not working on this currently. Anyone who wishes to take this up, please feel free to do so.

[Aniket21mathur]

@quozl The problem is that I haven't found the substitute for telepathy.server.DBusProperty. I've looking here: https://lazka.github.io/pgi-docs/#TelepathyGLib-0.12. Any suggestion?

Can we use this TP_IFACE_DBUS_PROPERTIES as a replacement for from telepathy.server import DBusProperties. I found it in the telepathy-glib documentation. Thanks!

[Aniket21mathur]

@quozl After the latest commit the error is not longer shown. Can you confirm it?

The commit that Fixed this traceback, again made dependency on telepathy with the import of Channel from telepathy.server.

[quozl]

Can we use this TP_IFACE_DBUS_PROPERTIES as a replacement for from telepathy.server import DBusProperties. I found it in the telepathy-glib documentation. Thanks!

I use the sources and compare them.

sudo apt source python-telepathy
sudo apt source gir1.2-telepathyglib-0.12

telepathy.server.DBusProperties is defined in line 29 of telepathy-python-0.15.19/src/server/properties.py as a class with inheritance from dbus.service.Interface, in turn from the python-dbus package, a static binding to the D-Bus API, library libdbus. Is verified by interactive Python;

from telepathy.server import DBusProperties
help(DBusProperties)
x = DBusProperties()
dir(x)
type(x)

Now activity.py has a class _ClientHandler which inherits from DBusProperties and calls the method _implement_property_get in that subclass. So anything you look for to replace this class without changing code has to have that method or provide another method to add D-Bus properties.

On the other hand, TP_IFACE_DBUS_PROPERTIES is a string constant, or quark, with a value "org.freedesktop.DBus.Properties", according to line 25070 of telepathy-glib-0.24.1/telepathy-glib/TelepathyGLib-0.12.gir. Consistent with the documentation you found, which says "_Expands to a call to a function that returns a quark for the interface name "org.freedesktop.DBus.Properties" _"

from gi.repository import TelepathyGLib
x = TelepathyGLib.IFACE_DBUS_PROPERTIES
type(x)
x

A string constant isn't the same as the DBusProperties class, so the answer is "no."

Inheritance and large blocks of code in _ClientHandler may need to be replaced. And our code should probably also switch from import dbus to from gi.repository import DBus or DBusGLib, depending on which is appropriate for the task.

Step back and ask yourself; why does the _ClientHandler class exist, and what is it for?

[quozl]

You may also be interested in reviewing code for the two source packages that rely on TelepathyGLib in Ubuntu 18.04; they are gnome-shell and polari. Both are written in JavaScript and use GObject Introspection APIs.

[quozl]

I've a notification but can't find the message here; a question was how to add these commits to a local branch; my answer is to bring the commits into your local repository then add them to the index;

git fetch origin pull/401/head
git cherry-pick H1 H2 ...

Where H1 and H2 are commit hashes listed in the pull request.

[Aniket21mathur]

Thanks. I just found a way after commenting so I deleted the comment. I did git fetch upstream pull/401/HEAD:Port .

[quozl]

Replaced by #412. Thanks to @pro-panda for the work this time last year, and for @tonadev's work since.