Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'org.gnome.Hamster.GUI' - strange semantics of window pop-up and persistence #563

Closed
mwilck opened this issue Feb 14, 2020 · 6 comments
Closed

Comments

@mwilck
Copy link
Contributor

mwilck commented Feb 14, 2020

The org.gnome.Hamster.GUI service is supposed to replace the functionality of org.gnome.Hamster.WindowsServer. I find its usefulness limited. Here's why.

The following 3 python statements cause a hamster window to appear:

import dbus
sb = dbus.SessionBus()
po = sb.get_object('org.gnome.Hamster.GUI', '/org/gnome/Hamster/GUI')

IOW, Just creating a proxy object for org.gnome.Hamster.GUI opens a window.
This is unexpected. I would expect that the application window would only open after I run
the Activate method of the org.gtk.Application interface, or activate a specific action via org.gtk.Actions. The same happens when using d-feet: Simply clicking on the org.gnome.Hamster.GUI address will cause the hamster window to appear. I've never seen this with any other DBus interface, and strongly believe that it's wrong. I just tried with a couple of other services that offer the org.gtk.Application API; I've found no other application that behaves like this.

In particular, this is unwanted if the org.gnome.hamster.GUI API is supposed to be used by other applications, for example the GNOME shell plugin. The plugin can't invoke the "add" action (showing the "edit activity" window) without showing the "overview" window first. If I continue the example above like this:

actions = dbus.Interface(po, 'org.gtk.Actions')
actions.Activate("add", [], [])

I get both windows, the overview and the "edit activity" window. For users of the GNOME shell extension and similar applications, this is highly confusing and distracting.

But what's worse, closing the hamster window also exits hamster and thus the DBus service, and attempting to access it will cause an error:

po = sb.get_object('org.gnome.Hamster.GUI', '/org/gnome/Hamster/GUI')
# overview window opens 
# ** user closes the window **
actions = dbus.Interface(po, 'org.gtk.Actions')
actions.List()
File "<stdin>", line 1, in <module>
File "/usr/lib/python3.7/site-packages/dbus/proxies.py", line 72, in __call__
    return self._proxy_method(*args, **keywords)
 File "/usr/lib/python3.7/site-packages/dbus/proxies.py", line 147, in __call__
     **keywords)
  File "/usr/lib/python3.7/site-packages/dbus/connection.py", line 653, in call_blocking message, timeout)
dbus.exceptions.DBusException: org.freedesktop.DBus.Error.ServiceUnknown: The name :1.151 was not provided by any .service files

This is horrible for any application trying to use the DBus interface. The proxy object (po in my example) continue to exist, but trying to call a method on it fails with a DBus error. Thus applications can't "cache" proxy objects in python variables, because these objects might be invalidated any time.

Compare this to the legacy hamster windows service, which runs in the background and provides dedicated methods to open certain windows to manipulate the hamster database. A proxy object opened to its address will persist until the session ends or someone forcibly kills the service, which is what I'd expect from an application that offers DBus services.

IMO this is how the new "GUI" service should also be designed. It should run in the background, should not quit when the associated window is closed, and should open dedicated windows when asked for it by a DBus method call.

The way this is currently designed may have its merits that I currently don't see. But I am quite certain that this design is not suitable to replace hamster-windows-service.

@ederag
Copy link
Collaborator

ederag commented Feb 14, 2020

TL;DR thoroughly.
That's just because you want to use that now,
without any knowledge of the new wayI mentioned in #549 (comment).
That's one of the reasons why the Application can not be a simple replacement
for the org.gnome.Hamster.WindowService, and why we temporarily need 3 servers...

Please wait before looking into that again. I'll give pointers later.

@ederag ederag closed this as completed Feb 14, 2020
@mwilck
Copy link
Contributor Author

mwilck commented Feb 14, 2020

@ederag, you shouldn't dismiss this without even looking at it. It's not only disrespectful against myself, I'm pretty sure it's against your own interests. And no, this is not only about hamster-windows-service and the shell extension.

Show me one single other DBus interface designed like this. And explain to me how this API would ever be used, for any purpose, by any application (or, in case you agree it's unusable, explain to me why this API exists at all).

I understand that you're busy, you don't have to look at this now, but I wish you'd fix the design before 3.0. It will be harder to fix it afterwards.

@ederag
Copy link
Collaborator

ederag commented Feb 14, 2020

@mwilck #549 (comment) gave some background for the decision to get this out of sight quickly,
as #549 already made the pressure too high,
but I admit that this comment was stern, sincerely sorry about that.

But despite the pressure and the fact that #549 (comment) had been posted 30 minutes earlier (messages probably crossed), I did take time to answer accurately
(the header says "too long, did not read thoroughly", but I did read and think before posting).

And that in particular was accurate:

Application [meaning the org.gnome.Hamster.GUI] can not be a simple replacement for the org.gnome.Hamster.WindowService

Hence my post was all about org.gnome.Hamster.GUI, totally relevant, to the point,
and not hastily dismissive at all.

And no, this is not only about hamster-windows-service and the shell extension.

The shell extension is not mentioned in my message either.

So there was absolutely no "disrespect" from me at all, on the contrary, given the circumstances.
[although I fully understand why it might have looked otherwise given the misunderstanding
and sternness of the message, sorry again for that]

Show me one single other DBus interface designed like this.

There are none, because the new gnome way is not a DBus interface we are used to.
They did not even give a way to create methods for the service.

I'll provide links and details tomorrow in a separate issue,
if you can promise not to start a polemic over there,
and just wait for others to raise concerns if there are any ? (cf. #549 (comment))
I really need to focus on code now.

Finally, can't let that pass:

I'm pretty sure it's against your own interests.

It's all about the project here. We both care about it, maybe too much ?

@ederag
Copy link
Collaborator

ederag commented Feb 16, 2020

I'll provide links and details tomorrow in a separate issue

Well, it's been there all along:
https://github.com/mwilck/hamster/blob/06c42d2655d1d1c79c67f12c01522d317b16506b/src/hamster-windows-service.py#L21-L26

I recently changed the phrasing (d7badae)
because the interface may change.

Note: that's an interface I want to experiment with at my pace,
to really understand it before changing the subprocess system to the new Gio.DBusActionGroup.
I'll do the migration myself when ready to.

@mwilck
Copy link
Contributor Author

mwilck commented Feb 17, 2020

Well, it's been there all along: [...]

Yes I'd seen that, and actually it was the very reason why I opened this issue.

Unless I hopelessly misunderstand, the comment you linked to above recommends to use the org.gtk.Actions DBus API that your new code created. I tried to experiment with that, and found it unusable, for the reasons I pointed out in the issue description. IMHO you need to fix that before anyone can seriously consider coding an application that uses this API. And because APIs should be stable, I would like to see this fixed before an 3.0 final release. If you don't want to do that (understandable), perhaps consider hiding the API for the time being.

Summing up again:

  1. Simply accessing the DBus object starts hamster and opens an overview window. I am 95% positive that that's not how it should be.
  2. Multiple windows appear if e.g. the "add" action is called.
  3. If the user closes the window(s), existing DBus proxy objects fail to work because their connection endpoint is closed.

There are none, because the new gnome way is not a DBus interface we are used to.

I've been looking around on my desktop with D-feet. There are actually a few applications providing org.gtk.Application and associated org.gtk.Actions APIs. Examples would be org.freedesktop.PackageKit or org.gnome.DiskUtils. The ones I tried create windows only when an actual Action is called.

OTOH, various core GNOME components appear not to have switched to this API yet. I see no urge for hamster to do so.

Side note: Actually I have some trouble understanding the value-add of this API design. It mainly seems to introduce a new level of API introspection, which was already available on DBus anyway via org.freedesktop.DBus.Introspectable. On the flip side, it replaces clean method invocations by "Actions" whose parameters are hidden in an obscure "Array of [Variant]". I'm certainly overlooking something very important, but at first sight it doesn't look like an improvement to me.

@ederag
Copy link
Collaborator

ederag commented Feb 17, 2020

There are actually a few applications providing org.gtk.Application and associated org.gtk.Actions APIs. Examples would be org.freedesktop.PackageKit or org.gnome.DiskUtils

Thanks for the examples.

various core GNOME components appear not to have switched to this API yet. I see no urge for hamster to do so.

There's no hurry at all.

Actually we are taking the most open route:
Keep the stable org.gnome.Hamster.WindowServer as the main interface.
Begin to experiment with the org.gnome.Hamster.GUI and Gio.DBusActionGroup

b0eac53 makes that even clearer,
I just forgot about this line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants