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.

Sign up for GitHub

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

Jump to bottom

Restore shutdown #631

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Restore shutdown #631

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
208073d
Restore stop on shutdown settings
sgranjoux Sep 20, 2020
bb05f8b
Restore stop tracking on log out
sgranjoux Sep 20, 2020
f8f0ce1
Avoid error when GNOME session is not available
sgranjoux Jul 3, 2021
c903aa2
Remove tooltip and warning message
sgranjoux Jul 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions data/org.gnome.hamster.gschema.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,14 @@
The folder the last report was saved to
</description>
</key>

<key type="b" name="stop-on-shutdown">
<default>false</default>
<summary>Stop tracking on shutdown</summary>
<description>
Stop tracking current activity on shutdown
</description>
</key>

<key type="u" name="day-start-minutes">
<default>330</default>
Expand Down
20 changes: 19 additions & 1 deletion data/preferences.ui
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,24 @@
<property name="valign">start</property>
<property name="orientation">vertical</property>
<property name="spacing">8</property>
<child>
<object class="GtkCheckButton" id="shutdown_track">
<property name="label" translatable="yes">Stop tracking on shutdown</property>
<property name="visible">True</property>
<property name="can_focus">True</property>
<property name="receives_default">False</property>
<property name="use_underline">True</property>
<property name="xalign">0.5</property>
<property name="draw_indicator">True</property>
<property name="has_tooltip">True</property>
<property name="tooltip_text" translatable="yes">Work only on GNOME</property>
sgranjoux marked this conversation as resolved.
Show resolved Hide resolved
</object>
<packing>
<property name="expand">False</property>
<property name="fill">True</property>
<property name="position">0</property>
</packing>
</child>
<child>
<object class="GtkBox" id="day-start box">
<property name="visible">True</property>
Expand All @@ -54,7 +72,7 @@
<property name="expand">False</property>
<property name="fill">True</property>
<property name="padding">4</property>
<property name="position">0</property>
<property name="position">1</property>
</packing>
</child>
<child>
Expand Down
11 changes: 10 additions & 1 deletion src/hamster-service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
i18n.setup_i18n() # noqa: E402

from hamster.storage import db
from hamster.session import DbusSessionListener
from hamster.lib import datetime as dt
from hamster.lib import default_logger
from hamster.lib.dbus import (
Expand All @@ -26,6 +27,7 @@
to_dbus_fact_json
)
from hamster.lib.fact import Fact, FactError
from hamster.lib.configuration import conf

logger = default_logger(__file__)

Expand All @@ -38,7 +40,7 @@
quit()


class Storage(db.Storage, dbus.service.Object):
class Storage(db.Storage, dbus.service.Object, DbusSessionListener):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if adding a mixin class here is the cleanest way to implement this. Also, it doesn't seem a responsibility of a class called Storage to implement a "stop-tracking-on-shutdown", but maybe that class already does more than its name implies (haven't looked too closely now).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with that. It's probably the simplest way but not the cleanest one.

I think I can create a separate object having a reference on the storage object to call StopTracking. To make it even more separated, this object can send a stop tracking message on the dbus, it could be even a separate process.

__dbus_object_path__ = "/org/gnome/Hamster"

def __init__(self, loop):
Expand All @@ -50,6 +52,7 @@ def __init__(self, loop):
db.Storage.__init__(self, unsorted_localized="")

self.mainloop = loop
DbusSessionListener.__init__(self)

self.__file = gio.File.new_for_path(__file__)
self.__monitor = self.__file.monitor_file(gio.FileMonitorFlags.WATCH_MOUNTS | \
Expand Down Expand Up @@ -79,6 +82,12 @@ def run_fixtures(self):
self.add_activity(activity, cat_id)


# stop current task on log out if requested
def end_session(self):
"""Stop tracking on logout."""
if conf.get("stop-on-shutdown"):
self.stop_tracking(None)

# stop service when we have been updated (will be brought back in next call)
# anyway. should make updating simpler
def _on_us_change(self, monitor, gio_file, event_uri, event):
Expand Down
2 changes: 2 additions & 0 deletions src/hamster/preferences.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,8 @@ def show(self):
self.window.show_all()

def load_config(self, *args):
conf.bind("stop-on-shutdown", self.get_widget("shutdown_track"), "active")

self.day_start.time = conf.day_start

self.tags = [tag["name"] for tag in runtime.storage.get_tags(only_autocomplete=True)]
Expand Down
85 changes: 85 additions & 0 deletions src/hamster/session.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
# -*- coding: utf-8 -*-

# Copyright (C) 2020 Sébastien Granjoux <[email protected]>

# This file is part of Project Hamster.

# Project Hamster is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.

# Project Hamster is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.

# You should have received a copy of the GNU General Public License
# along with Project Hamster. If not, see <http://www.gnu.org/licenses/>.

import dbus
import logging
logger = logging.getLogger(__name__)

class DbusSessionListener(object):
"""Listen for GNOME manager end session event."""


def end_session(self):
"""Override this method to do something at the end of a session.
It must not attempt to interact with the user and will be given
a maximum of ten seconds to perform the actions."""
pass
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this should raise NotImplementedError?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think pass is fine, there is no need to do something.
Eventually, I could extend this object to provide a hook in query_end_session_handler. I will allow to use it to delay the end of a session and it could be that nothing is needed when the session end. Anyway, it's not done currently, so I could raise NotImplementedError if you think it's better.



def __init__(self):
"""Connect to the GNOME manager session signals"""

# Get SessionManager interface
session_bus = dbus.SessionBus()
try:
session_manager = session_bus.get_object("org.gnome.SessionManager",
"/org/gnome/SessionManager")
self.__session_manager_iface = dbus.Interface(session_manager,
dbus_interface="org.gnome.SessionManager")
self.__client_id = self.__session_manager_iface.RegisterClient("", "")

# Get SessionManager.ClientPrivate interface
session_client = session_bus.get_object("org.gnome.SessionManager",
self.__client_id)
self.__session_client_private_iface = dbus.Interface(session_client,
dbus_interface="org.gnome.SessionManager.ClientPrivate")

# Connect to the needed signals
session_bus.add_signal_receiver(self.__query_end_session_handler,
signal_name = "QueryEndSession",
dbus_interface = "org.gnome.SessionManager.ClientPrivate",
bus_name = "org.gnome.SessionManager")

session_bus.add_signal_receiver(self.__end_session_handler,
signal_name = "EndSession",
dbus_interface = "org.gnome.SessionManager.ClientPrivate",
bus_name = "org.gnome.SessionManager")

session_bus.add_signal_receiver(self.__stop_handler,
signal_name = "Stop",
dbus_interface = "org.gnome.SessionManager.ClientPrivate",
bus_name = "org.gnome.SessionManager")
except dbus.exceptions.DBusException:
logger.warning("Unable to connect to GNOME session manager, stop trcking on logout won't work")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please either make this logger.info() or just replace this line with pass so command line users don't see this message on every single command (because warning is the default logging level). Also trcking -> tracking.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I have changed it but I think this message will appear only when your start the hamster daemon so probably not on each command.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, I forgot that I was using a cover script that restart the daemon before each command. Thanks anyway.


def __query_end_session_handler(self, flags):
"""Inform that the session is about to end. It must reply with
EndSessionResponse within one second telling if it is ok to proceed
or not and why. If flags is true, the session will end anyway."""
self.__session_client_private_iface.EndSessionResponse(True, "")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this handler needed? If we're not doing anything other than saying it is ok to end the session, isn't this just the default? Or is it so that if you register something (I'm not sure about right terminology here) against SessionManager, that you need to also handle this QueryEndSession event?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not checked, but according to the comment I have understood that when you register against the SessionManager you have to handle this message.
If it's not done, I think it can delay the log out by 1 seconds, eventually the session manager could remove the corresponding program and we will not get the end session message.


def __end_session_handler(self, flags):
"""Inform that the session is about to end. It must reply with
EndSessionResponse within ten seconds."""
self.end_session()
self.__session_client_private_iface.EndSessionResponse(True, "")

def __stop_handler(self):
"""Remove from the session."""
self.__session_manager_iface.UnregisterClient(self.__client_id)