Add automatic backup of the queue. Also, signal handling #19
Conversation
There was a problem hiding this comment.
While I think some users will welcome the function of periodic persistence of notifications I would like to error on the side of doing less by default and have it be an opt-in feature.
The code reformatting looks good to me, makes things more readable, although I do not work with Python very often.
rofication/_dbus.py
Outdated
| self._object = RoficationDbusObject(queue) | ||
| # create GLib mainloop, this is needed to make D-Bus work and takes care of catching signals. | ||
| self._loop = MainLoop() | ||
|
|
||
| def run(self) -> None: | ||
| timeout_add_seconds(60, self.backup_queue) |
There was a problem hiding this comment.
I would prefer it if this behavior is opt-in. It's a matter of opinion either way but I reckon that most users would not want this functionality as it comes at the cost of periodic disk writes.
There was a problem hiding this comment.
I disagree with those odds :-) I definitely prefer some I/O to notification loss. Note that the write only happens if there are unsaved notifications.
Anyway, I'm made it configurable with default to no save. I know that I will keep my notifications safe, but well :-)
|
|
||
| from ._metadata import ROFICATION_VERSION, ROFICATION_NAME, ROFICATION_URL | ||
| from ._notification import Notification, Urgency | ||
| from ._queue import NotificationQueue | ||
| from datetime import datetime | ||
|
|
||
| NOTIFICATIONS_DBUS_INTERFACE = 'org.freedesktop.Notifications' | ||
| NOTIFICATIONS_DBUS_OBJECT_PATH = '/org/freedesktop/Notifications' | ||
| NOTIFICATIONS_DBUS_INTERFACE = "org.freedesktop.Notifications" |
There was a problem hiding this comment.
I'm not sure about the single quote vs double. Does this change make a syntactic difference or simply style?
There was a problem hiding this comment.
It's style, but it comes from here:
https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html
Black is an opinionated Python formatter - quite in use these days, as it solves all the religious format wars by imposing one style :-)
So just running black on a set of files will give you consistency across the code.
| filename = self._queue_file | ||
|
|
||
| if not filename: | ||
| warn("Unable to save notification queue, no file specified") |
There was a problem hiding this comment.
Will this emit in syslog or somewhere else user visible?
There was a problem hiding this comment.
I'm using warn there as that's what _queue.py already had for other messages. In any case, its output just go to stderr.
| try: | ||
| print('Saving notification queue to file') | ||
| with open(filename, 'w') as f: | ||
| print(f"Saving notification queue to file: {filename}") |
Currently the queue is only saved when the program ends cleanly. But on crash, system reboot, etc, that never happens, and then system comes up with whatever we had on the queue when it was last saved.
This PR does two things:
Tested by running the daemon from terminal, sending it SIGTERM, also keyboard interrupt, send notifications every few seconds to make the queue dirty and force a save, then don't send any additional notification to verify that it won't save unless needed.
Bonus: Also run black, which is why there's a few format changes in the files.