WIP: No dbus #573
WIP: No dbus #573
Conversation
To be consistent with org.gnome.Hamster.GUI.
To be case-consistent with HamsterGUI.
Soft link back to lib/dbus.py. Waf will install a plain file instead of a link. No big deal, and the note should help in case of confusion.
The Storage is passed along, instead of using a new client or runtime each time.
Deprecate RuntimeStore and runtime.
|
I don't like this idea. dbus serves as a layer of separation/abstraction between the database and the clients using it. Unless I'm very mistaken, that's a basic design principle of hamster. IMO the rationale in the PR is not strong enough to justify such a massive architectural change. If anything, we should rather follow @elbenfreund's idea to separate out |
|
That was part of the ongoing plan, as I said,
I know you disagree with the plan.
Sure, that stays the default.
|
|
@mwilck Unlike you, I think having a --no-dbus option is a great idea.
Hamster itself works fine without dbus. (In fact I'm working slowly on my own hamster-lite fork in which I've ripped out the dbus support altogether). With hamster brought up by a desktop hotkey and closed by the ESC key, hamster usage is super efficient and out-of-your-face. Not all Hamster users are gnome-hamster-extension centric! |
|
@GeraldJansen, dbus is in no way tied to the GNOME extension. I believe you are aware of that. The usage of dbus is motivated by the idea of separating and abstracting tasks. By using DBus, end-user applications don't have to worry about database access, transactions, synchronization, concurrency etc., because there's only one application (the DBus service) that accesses the data base. It's actually beneficial for development, because data base access and UI functionality are separated and can be developed and tested separately. With this separation, the DBus layer allows easy development of "foreign" UI clients. The GNOME extension is just one example. IIRC you said you use the XFCE extension, which uses the same functionality. With the invention of the "hamster windows service", the original hamster authors went one step further and made it possible to create a new UI without implementing every dialog. You can implement the basic UI, and hand over to the original hamster for details. That was true genius and made the invention of the GNOME extension and other helpers feasible in the first place. By ditching this concept, your "hamster-lite fork" becomes a very different type of application. That doesn't mean "inferior" or "worse", but quite far away from the original spirit in which hamster has been designed, as I understand it.
You and @ederag seem to see DBus mainly as a source of complication and bugs. I concur that it adds complexity if you want to pull up new UI features quickly. Rather than simply hacking a new field in the data base and adding an UI control for it, you have to consider how to transport the new piece of information through the available DBus methods, or possibly extend the DBus API, which is additional work and requires a lot of careful consideration. I still believe that it's good to have this separation, and that spending the additional effort would pay off in the long run. What you are referring to in the two bullets above actually IMO shows that we have too little separation between frontend and backend in the current code, not too much.
In which way is this related to DBus? Doing it needs a change to the storage layer in the first place. DBus could actually be helpful here, as the layers on top of the DBus protocol could be agnostic of how many database files the storage layer accesses, and where they are stored, and let the storage layer worry about it alone. I'm not sure what you mean with "database merging" - do you want to merge the storage in a one-shot operation, or do you want to able to use both databases concurrently and sort of merge them transparently / on the fly? In the former case, I suppose a dedicated command line tool
Every modern desktop has some sort of messaging system that would be usable as a separation layer. It doesn't have to be DBus, of course. I'm not opposed to generalizing the concept, if someone wants to tackle that. |
|
As far as I know, Gnome-hamster-extension is the only really "alive" project that relies on the hamster dbus services, aside from Hamster's own CLI. (Btw, I do not use the xfce plugin, I just did some testing of it as a contributor to this project.) This PR is about adding an option to make it possible to use Hamster, including the GUI and CLI, without DBus, not about removing DBUS from Hamster. Unlike me, @ederag has always been a very strong supporter of maintaining a stable DBus interface. The dbus services should not be a layer between the main UI and the DB backend(s). In fact, in contrast to what you have stated earlier, it seems to me that @elbenfreund designed hamster_cli and hamster_gtk to be based directly on hamster_lib, and he added hamster_dbus later to support external service consumers like gnome-hamster-extension. For some background on the issue of accessing more than one DB and merging DBs, see #340, #570, and the external tool I developed htool Imho, it would be better to have the tool integrated in the Hamster CLI (for better visibility and maintainance), and I had to do some convincing to bring @ederag around to agree. I don't wish to argue endlessly, but it would be nice if you could be a bit more generous in your consideration of other's opinions. |
I understand. My concern is that, once that door is open, people will slip through it rather than considering the more stony DBus path, and the DBus API will bit rot over time.
Interesting, thanks for pointing this out. I suppose that warrants a discussion about the general architecture we aim at.
I have stated my opinion, and tried to explain my reasons. That doesn't mean I don't respect other people's opinions. You are free to try and convince me; your argument about |
|
(I typed most of this before the last two comments were posted, so I might need to separately respond to some of that, but no time right now). Let me add my view on this as well. As for the feature itself: Making hamster run without dbus, accessing the db storage directly, I can see merit in there. I do not think it should the only mode supported, or the default, but it I can see the merit for e.g. systems without dbus and local development and testing. To properly support both, we would need a proper storage abstraction, to make sure that most code does not actually have to know whether it talks to dbus or the database directly. When I started writing this comment, I expected this would require significant refactoring to achieve, but now I looked at some of the commits in this PR (in particular f8b09e0 that seems to be the core of this commit), we actually have most, if not all, of this abstraction in place already with So maybe adding As for needing As for this particular PR, it is quite big. However, it does seem that most changes are very cleanly separated into commits, which helps review. Still, for a lot of commits it is not immediately clear to me what their purpose is exactly (i.e. why are they needed or helpful to work towards Then, even with this PR improved for easier review, it is still a quite non-trivial change. Personally, I might not find enough time to properly review and test this to merge it (that is, I think there might be other (more pressing or more personally motivating) things I would rather spend my available Hamster time on. I suspect the same might hold for the rest of the maintainer team. Then, it seems @ederag has also lost interest in further contributions (unless I'm misinterpreting that comment), so I don't see this PR moving forward anytime soon. |
IMO this works only if we strictly enforce any additions to be made in a way that works through DBus, too; otherwise the DBus interface will fall behind. The non-native clients mostly aren't written in Python. In other languages, the "storage abstraction" used is exactly the |
Yeah, as I said, I think the dbus interface should just remain the default. And both
I think this change would only affect the main hamster app (cli and gui), not any of these non-native clients (which would of course not work in setups without dbus, but you can't have your cake and eat it, I guess). |
|
Hello, Thanks to this PR I'm able to run PS : I do not know if there is a more recent version (commit / release) which brings this option ? |
Not in this repo, to my knowledge. If you are new to hamster Development is super slow now. It's good and stable for me (also contains #597 and more), but no-one else is testing it. |
First part of #570, new option
The main interest is to bypass D-Bus for testing or if a #477-like happens again.
It could clearly have been done more concisely, but hey, there it is 🙂
The only breaking change is passing
storageas an argument instead of a centralizedruntime.But AFAIK no-one is using controllers or widgets.
Please chime in quickly if this is an overlook.
That change will (not done yet) allow to select a different database path for the storage
(e.g. to export as tsv and pipe to
diff).runtimewill stay there deprecated for a while to ensure compatibilty.All changes that could have affected known companions
have received backward compatibility measures. Please chime in quickly if there was a leftover
Still WIP, not well tested, for information.