refactor: extract config to separate module.

pomito/config.py

@@ -0,0 +1,52 @@
+# -*- coding: utf-8 -*-
+"""Configuration for pomito."""
+import logging
+import os
+from configparser import ConfigParser
+
+logger = logging.getLogger("pomito.config")
+
+
+class Configuration(object):
+    """Configuration settings for pomito.
+
+    Configuration is read from `config.ini` file in pomito data directory. See
+    documentation for more details.
+    """
+
+    # Set sensible defaults
+    session_duration = 25 * 60      # Pomodoro session duration
+    short_break_duration = 5 * 60
+    long_break_duration = 15 * 60
+    long_break_frequency = 4        # A long break after 4 sessions
+    ui_plugin = "qtapp"
+    task_plugin = "nulltask"
+
+    def __init__(self, config_file):
+        """Create an instance of pomito configuration."""
+        self._config_file = config_file
+        self._parser = ConfigParser()
+
+    def get_setting(self, plugin):
+        """Get setting for a plugin."""
+        if self._parser.has_section(plugin):
+            return self._parser.items(plugin)
+        return []
+
+    def load(self):
+        """Parse the pomito user configuration file."""
+        config_file = self._config_file
+        if config_file is None or not os.path.isfile(config_file):
+            logger.info("Config file '{0}' not found. Using defaults.".format(config_file))
+            return
+
+        self._parser.read(config_file)
+        if self._parser.has_section("pomito"):
+            self.session_duration = self._parser.getint("pomito", "session_duration") * 60
+            self.short_break_duration = self._parser.getint("pomito", "short_break_duration") * 60
+            self.long_break_duration = self._parser.getint("pomito", "long_break_duration") * 60
+            self.long_break_frequency = self._parser.getint("pomito", "long_break_frequency")
+
+        if self._parser.has_section("plugins"):
+            self.ui_plugin = self._parser.get("plugins", "ui")
+            self.task_plugin = self._parser.get("plugins", "task")

pomito/main.py

@@ -11,6 +11,7 @@
 from peewee import SqliteDatabase
 
 import pomito.plugins
+from pomito.config import Configuration
 
 PACKAGE_NAME = "pomito"
 DATA_HOME = CONFIG_HOME = os.path.expanduser("~")
@@ -117,26 +118,21 @@ def __init__(self, config_file=None, database=None,
             database    peewee.SqliteDatabase database to use for tasks etc.
             create_message_dispatcher function creates a MessageDispatcher
         """
-        from configparser import SafeConfigParser
         from pomito import pomodoro
 
         self._config_file = config_file
         self._database = database
-        self._parser = SafeConfigParser()
         self._message_dispatcher = create_message_dispatcher()
         self._threads = {}
         self._plugins = {}
         self._hooks = []
 
         # Set default options
-        self.session_duration = 25 * 60      # Duration of a pomodoro session
-        self.short_break_duration = 5 * 60   # Duration of a short break between two sessions
-        self.long_break_duration = 15 * 60   # Duration of a longer break after every 4 sessions
-        self.long_break_frequency = 4        # Frequency of long breaks
         self._plugins['ui'] = 'qtapp'
         self._plugins['task'] = 'nulltask'
 
-        self._parse_config_file(self._config_file)
+        self._config = Configuration(config_file)
+        self._config.load()
 
         # Pomodoro service instance. Order of initializations are important
         self.pomodoro_service = pomodoro.Pomodoro(self)
@@ -193,54 +189,25 @@ def exit(self):
             self._message_dispatcher.join()
         for hook in self._hooks:
             hook.close()
-        #self._validate_state()
-        #self._parser.write()
+        # self._validate_state()
         if self._database is not None:
             self._database.close()
         return
 
     def get_db(self):
-        """Gets the database object.
+        """Get the database object.
 
         Returns:
             database peewee.SqliteDatabase object
         """
         return self._database
 
-    def get_parser(self):
-        """Gets the parser object.
-
-        Returns:
-            parser ConfigParser
-        """
-        return self._parser
+    def get_configuration(self):
+        return self._config
 
     def queue_signal(self, message):
         self._message_dispatcher.queue_message(message)
 
-    def _parse_config_file(self, config_file):
-        """Parse the pomito user configuration file. Sample config at
-        $Src/docs/sample_config.ini.
-
-        Args:
-            config_file: string Path to the configuration file
-        """
-        if config_file is None or not os.path.isfile(config_file):
-            logger.info("Config file '{0}' not found. Using defaults.".format(config_file))
-            return
-
-        self._parser.read(config_file)
-        if self._parser.has_section('pomito'):
-            self.session_duration = self._parser.getint('pomito', 'session_duration') * 60
-            self.short_break_duration = self._parser.getint('pomito', 'short_break_duration') * 60
-            self.long_break_duration = self._parser.getint('pomito', 'long_break_duration') * 60
-            self.long_break_frequency = self._parser.getint('pomito', 'long_break_frequency') * 60
-
-        if self._parser.has_section('plugins'):
-            self._plugins['ui'] = self._parser.get('plugins', 'ui')
-            self._plugins['task'] = self._parser.get('plugins', 'task')
-        return
-
     def _validate_state(self):
         """Validates configuration, plugins."""
         import pomito.plugins

pomito/plugins/task/trello.py

@@ -34,18 +34,18 @@ def __init__(self, pomodoro_service, get_trello_api=_create_trello_client):
 
     def initialize(self):
         """Initialize the trello task plugin."""
-        try:
-            def _get_config(config):
-                return self._pomodoro_service.get_config("task.trello", config)
-
-            api_key = _get_config("api_key")
-            api_secret = _get_config("api_secret")
-            self.trello_board = _get_config("board")
-            self.trello_list = _get_config("list")
-
-            self.trello_api = self._get_trello_client(api_key, api_secret)
-        except Exception as ex:
-            logger.error("Error initializing plugin: {0}".format(ex))
+        def _get_config(config):
+            return self._pomodoro_service.get_config("task.trello", config)
+
+        api_key = _get_config("api_key")
+        api_secret = _get_config("api_secret")
+        self.trello_board = _get_config("board")
+        self.trello_list = _get_config("list")
+
+        self.trello_api = self._get_trello_client(api_key, api_secret)
+        if api_key is None or api_secret is None\
+                or self.trello_board is None or self.trello_list is None:
+            logger.error("Error initializing plugin: invalid configuration")
 
     def get_tasks(self):
         """Get all incomplete tasks assigned to the user."""

pomito/pomodoro.py

@@ -1,6 +1,6 @@
 # -*- coding: utf-8 -*-
 # Pomito - Pomodoro timer on steroids
-# Implementation of Pomodoro service layer
+"""Implementation of Pomodoro service layer."""
 
 import logging
 import threading
@@ -70,10 +70,12 @@ def _get_timer(duration, callback, interval=1):
         return Timer(duration, callback, interval)
 
     def __init__(self, pomito_instance, create_timer=_get_timer):
+        """Create an instance of the pomodoro service."""
         self._pomito_instance = pomito_instance
         self._session_count = 0
         self._create_timer = create_timer
-        self._timer = self._create_timer(self._pomito_instance.session_duration,
+        self._config = self._pomito_instance.get_configuration()
+        self._timer = self._create_timer(self._config.session_duration,
                                          self._update_state)
         # other values = "{short, long}_break", "interruption"
         self._timer_type = TimerType.SESSION
@@ -88,48 +90,55 @@ def _stop_timer(self):
         return
 
     def get_config(self, plugin_name, config_key):
-        """Gets the config dict for <plugin_name> from pomito ini file"""
-        return self._pomito_instance.get_parser().get(plugin_name, config_key)
+        """Get the config dict for <plugin_name> from pomito ini file."""
+        c = self._config.get_setting(plugin_name)
+        config_value = None
+        for kv in c:
+            if kv[0] == config_key:
+                config_value = kv[1]
+        return config_value
 
     def get_db(self):
-        """Gets the in application database for Pomito."""
+        """Get the in application database for Pomito."""
         return self._pomito_instance.get_db()
 
     def get_data_dir(self):
-        """Gets the data directory for pomito application."""
+        """Get the data directory for pomito application."""
         from .main import DATA_DIR
         return DATA_DIR
 
     def get_task_plugins(self):
-        """Gets list of all registered task plugins."""
+        """Get list of all registered task plugins."""
         plugins = get_plugins().items()
         return [v for k, v in plugins if isinstance(get_plugin(k), TaskPlugin)]
 
     def get_tasks(self):
-        """Gets all tasks in the current task plugin."""
+        """Get all tasks in the current task plugin."""
         return self._pomito_instance.task_plugin.get_tasks()
 
     def get_tasks_by_filter(self, task_filter):
-        """Gets all tasks with attributes matching a filter.
+        """Get all tasks with attributes matching a filter.
 
         Args:
             task_filter: string to match task attributes
 
-        See TaskPlugin.get_tasks_by_filter."""
+        See TaskPlugin.get_tasks_by_filter.
+        """
         return self._pomito_instance.task_plugin.get_tasks_by_filter(task_filter)
 
     def get_task_by_id(self, task_id):
-        """Gets all tasks with id matching task id.
+        """Get all tasks with id matching task id.
 
         Args:
             task_id: int to match task id
 
-        See TaskPlugin.get_task_by_id."""
+        See TaskPlugin.get_task_by_id.
+        """
         return self._pomito_instance\
             .task_plugin.get_task_by_id(task_id)
 
     def start_session(self, task):
-        """Starts a pomodoro session.
+        """Start a pomodoro session.
 
         Args:
             task: Task - A task object, to be performed during this session
@@ -139,43 +148,50 @@ def start_session(self, task):
 
         self.current_task = task
         self._timer_type = TimerType.SESSION
-        self._timer = self._create_timer(self._pomito_instance.session_duration,
+        self._timer = self._create_timer(self._config.session_duration,
                                          self._update_state)
         msg = Message(self.signal_session_started,
                       session_count=self._session_count,
-                      session_duration=self._pomito_instance.session_duration,
+                      session_duration=self._config.session_duration,
                       task=self.current_task)
         self._pomito_instance.queue_signal(msg)
         self._timer.start()
 
     def stop_session(self):
-        """Stops a pomodoro session."""
+        """Stop a pomodoro session."""
         self._stop_timer()
         self.current_task = None
 
     def start_break(self):
-        """Starts a break on completion of a session.
+        """Start a break on completion of a session.
 
         A short break of 5 minutes is introduced after each session.
         A longer break of duration 15 minutes is introduced after 4 consecutive
         sessions.
         """
-        if self._session_count == self._pomito_instance.long_break_frequency:
+        if self._session_count == self._config.long_break_frequency:
             self._timer_type = TimerType.LONG_BREAK
-            _duration = self._pomito_instance.long_break_duration
+            _duration = self._config.long_break_duration
         else:
             self._timer_type = TimerType.SHORT_BREAK
-            _duration = self._pomito_instance.short_break_duration
+            _duration = self._config.short_break_duration
         self._timer = self._create_timer(_duration, self._update_state)
         msg = Message(self.signal_break_started, break_type=self._timer_type)
         self._pomito_instance.queue_signal(msg)
         self._timer.start()
 
     def stop_break(self):
-        """Stops a break session."""
+        """Stop a break session."""
         self._stop_timer()
 
     def start_interruption(self, reason, is_external, add_unplanned_task):
+        """Start an interruption.
+
+        Args:
+            reason (str): reason for interruption
+            is_external (bool): True if this is an external interruption
+            add_unplanned_task (bool): Adds an unplanned task if True
+        """
         # TODO option to stop auto monitoring of interruptions
         # TODO add the interruption activity
         # TODO support interruption type for interruption_stop
@@ -190,11 +206,13 @@ def start_interruption(self, reason, is_external, add_unplanned_task):
         self._timer.start()
 
     def stop_interruption(self):
-        """Stops the interruption timer."""
+        """Stop the interruption timer."""
         self._stop_timer()
 
     def _update_state(self, notify_reason):
-        """This is called in context of timer thread. Try to keep execution as
+        """Update state of the timer.
+
+        This is called in context of timer thread. Try to keep execution as
         minimal as possible in case of increment notifications.
 
         This method queues the signals into the dispatcher queue.
@@ -235,8 +253,9 @@ def _update_state(self, notify_reason):
 
 
 class Timer(threading.Thread):
+    """A custom timer inspired by threading.Timer.
 
-    """Inspired by threading.Timer. Two major differences:
+    Two major differences:
         - We call the parent_callback every interval (default = one second)
         - We support notify_reason with callbacks. We support "interrupt" as
         reason because of support for other plugins and hooks which listen to
@@ -252,7 +271,8 @@ class Timer(threading.Thread):
     """
 
     def __init__(self, duration, callback, interval=1):
-        """
+        """Create an instance of the timer.
+
         Args:
             duration: total duration for the timer
             callback: callback that will be invoked on stop or completion
@@ -268,12 +288,14 @@ def __init__(self, duration, callback, interval=1):
         self._finished = threading.Event()
 
     def start(self):
+        """Start the timer."""
         if threading.currentThread() == self:
             raise RuntimeError("Cannot call start on the timer thread itself.")
         self._notify_reason = TimerChange.INCREMENT
         threading.Thread.start(self)
 
     def stop(self):
+        """Stop the timer."""
         if threading.currentThread() == self:
             raise RuntimeError("Cannot call stop on the timer thread itself.")
         self._notify_reason = TimerChange.INTERRUPT

tests/plugins/task/test_trello.py

@@ -84,7 +84,7 @@ def test_trello_can_create_trello_instance(trello):
 def test_initialize_logs_error_for_invalid_config(trello, caplog):
     trello.initialize()
 
-    err_msg = "Error initializing plugin: No option"
+    err_msg = "Error initializing plugin: invalid configuration"
     err_rec = [r for r in caplog.record_tuples if r[1] == logging.ERROR]
     assert 1 == len(err_rec)
     assert err_msg in err_rec[0][2]