Merge pull request #1045 from oddstr13/pr-cleanup-3

Misc cleanup and refractoring

.gitignore

@@ -70,6 +70,7 @@ Thumbs.db
 !.vscode/extensions.json
 !.vscode/settings.json
 pyinstrument/
+.venv/
 
 # Now managed by templates
 addon.xml

.vscode/extensions.json

@@ -1,6 +1,7 @@
 {
 	"recommendations": [
 		"ms-vscode-remote.remote-containers",
-		"ms-python.black-formatter"
+		"ms-python.black-formatter",
+		"ms-python.mypy-type-checker"
 	]
 }

jellyfin_kodi/entrypoint/service.py

@@ -10,6 +10,7 @@ from importlib import reload
 
 # Workaround for threads using datetime: _striptime is locked
 import _strptime  # noqa:F401
+
 import xbmc
 import xbmcgui
 

jellyfin_kodi/full_sync.py

@@ -13,7 +13,12 @@ from .objects import Movies, TVShows, MusicVideos, Music
 from .database import Database, get_sync, save_sync, jellyfin_db
 from .helper import translate, settings, window, progress, dialog, LazyLogger, xmls
 from .helper.utils import get_screensaver, set_screensaver
-from .helper.exceptions import LibraryException, PathValidationException
+from .helper.exceptions import (
+    LibraryException,
+    LibraryExitException,
+    LibrarySyncLaterException,
+    PathValidationException,
+)
 
 ##################################################################################################
 
@@ -169,11 +174,13 @@ class FullSync(object):
         selection = dialog("multi", translate(33120), choices)
 
         if selection is None:
-            raise LibraryException("LibrarySelection")
+            # TODO: Why are we handling these two differently?
+            # presumably one means the dialog got aborted, the other means that we just pressed ok without selecting any libraries
+            raise LibraryException("Library selection dialog returned None.")
         elif not selection:
             LOG.info("Nothing was selected.")
 
-            raise LibraryException("SyncLibraryLater")
+            raise LibrarySyncLaterException("No libraries were selected, sync later.")
 
         if 0 in selection:
             selection = list(range(1, len(libraries) + 1))
@@ -275,11 +282,13 @@ class FullSync(object):
 
                 media[library["CollectionType"]](library)
         except LibraryException as error:
-
+            # TODO: Fixme; We're catching all LibraryException here,
-            if error.status == "StopCalled":
+            # but silently ignoring any that isn't the exit condition.
+            # Investigate what would be appropriate behavior here.
+            if isinstance(error, LibraryExitException):
                 save_sync(self.sync)
-
                 raise
+            LOG.warning("Ignoring exception %s", error)
 
         except PathValidationException:
             raise

jellyfin_kodi/helper/api.py

@@ -1,11 +1,12 @@
 # -*- coding: utf-8 -*-
 from __future__ import division, absolute_import, print_function, unicode_literals
 
+import json
+
 ##################################################################################################
 
 from . import settings, LazyLogger
 from .utils import translate_path
-import json
 
 ##################################################################################################
 

jellyfin_kodi/helper/exceptions.py

@@ -19,9 +19,15 @@ class HTTPException(Exception):
 
 
 class LibraryException(Exception):
-    # Jellyfin library sync exception
+    pass
-    def __init__(self, status):
+
-        self.status = status
+
+class LibraryExitException(LibraryException):
+    "Exception raised to propagate application exit."
+
+
+class LibrarySyncLaterException(LibraryException):
+    "Raised when no libraries are selected for sync."
 
 
 class PathValidationException(Exception):
@@ -30,5 +36,3 @@ class PathValidationException(Exception):
 
     TODO: Investigate the usage of this to see if it can be done better.
     """
-
-    pass

jellyfin_kodi/helper/lazylogger.py

@@ -1,9 +1,12 @@
 # -*- coding: utf-8 -*-
 from __future__ import division, absolute_import, print_function, unicode_literals
 
+from typing import TYPE_CHECKING
+
 
 class LazyLogger(object):
-    """`helper.loghandler.getLogger()` is used everywhere.
+    """
+    `helper.loghandler.getLogger()` is used everywhere.
     This class helps to avoid import errors.
     """
 
@@ -19,3 +22,88 @@ class LazyLogger(object):
 
             self.__logger = getLogger(self.__logger_name)
         return getattr(self.__logger, name)
+
+    #####################################################################
+    # Following are stubs of methods provided by `logging.Logger`.      #
+    # Please ensure any actually functional code is above this comment. #
+    #####################################################################
+
+    if TYPE_CHECKING:
+
+        def setLevel(self, level):
+            """
+            Set the logging level of this logger.  level must be an int or a str.
+            """
+            ...
+
+        def debug(self, msg, *args, **kwargs):
+            """
+            Log 'msg % args' with severity 'DEBUG'.
+
+            To pass exception information, use the keyword argument exc_info with
+            a true value, e.g.
+
+            logger.debug("Houston, we have a %s", "thorny problem", exc_info=1)
+            """
+            ...
+
+        def info(self, msg, *args, **kwargs):
+            """
+            Log 'msg % args' with severity 'INFO'.
+
+            To pass exception information, use the keyword argument exc_info with
+            a true value, e.g.
+
+            logger.info("Houston, we have a %s", "interesting problem", exc_info=1)
+            """
+            ...
+
+        def warning(self, msg, *args, **kwargs):
+            """
+            Log 'msg % args' with severity 'WARNING'.
+
+            To pass exception information, use the keyword argument exc_info with
+            a true value, e.g.
+
+            logger.warning("Houston, we have a %s", "bit of a problem", exc_info=1)
+            """
+            ...
+
+        def error(self, msg, *args, **kwargs):
+            """
+            Log 'msg % args' with severity 'ERROR'.
+
+            To pass exception information, use the keyword argument exc_info with
+            a true value, e.g.
+
+            logger.error("Houston, we have a %s", "major problem", exc_info=1)
+            """
+            ...
+
+        def exception(self, msg, *args, exc_info=True, **kwargs):
+            """
+            Convenience method for logging an ERROR with exception information.
+            """
+            ...
+
+        def critical(self, msg, *args, **kwargs):
+            """
+            Log 'msg % args' with severity 'CRITICAL'.
+
+            To pass exception information, use the keyword argument exc_info with
+            a true value, e.g.
+
+            logger.critical("Houston, we have a %s", "major disaster", exc_info=1)
+            """
+            ...
+
+        def log(self, level, msg, *args, **kwargs):
+            """
+            Log 'msg % args' with the integer severity 'level'.
+
+            To pass exception information, use the keyword argument exc_info with
+            a true value, e.g.
+
+            logger.log(level, "We have a %s", "mysterious problem", exc_info=1)
+            """
+            ...

jellyfin_kodi/helper/utils.py

@@ -162,18 +162,6 @@ def dialog(dialog_type, *args, **kwargs):
     return types[dialog_type](*args, **kwargs)
 
 
-def should_stop():
-    """Checkpoint during the sync process."""
-    if xbmc.Monitor().waitForAbort(0.00001):
-        return True
-
-    if window("jellyfin_should_stop.bool"):
-        LOG.info("exiiiiitttinggg")
-        return True
-
-    return not window("jellyfin_online.bool")
-
-
 def get_screensaver():
     """Get the current screensaver value."""
     result = JSONRPC("Settings.getSettingValue").execute(

jellyfin_kodi/helper/wrapper.py

@@ -1,14 +1,17 @@
 # -*- coding: utf-8 -*-
 from __future__ import division, absolute_import, print_function, unicode_literals
 
+from functools import wraps
+
 #################################################################################################
 
 import xbmcgui
+import xbmc
 
 from . import LazyLogger
 
-from .utils import should_stop
+from .utils import window
-from .exceptions import LibraryException
+from .exceptions import LibraryExitException
 from .translate import translate
 
 #################################################################################################
@@ -22,6 +25,7 @@ def progress(message=None):
     """Will start and close the progress dialog."""
 
     def decorator(func):
+        @wraps(func)
         def wrapper(self, item=None, *args, **kwargs):
 
             dialog = xbmcgui.DialogProgressBG()
@@ -53,16 +57,18 @@ def progress(message=None):
 def stop(func):
     """Wrapper to catch exceptions and return using catch"""
 
+    @wraps(func)
     def wrapper(*args, **kwargs):
 
-        try:
+        if xbmc.Monitor().waitForAbort(0.00001):
-            if should_stop():  # ??? TODO: Fixme
+            raise LibraryExitException("Kodi aborted, exiting...")
-                raise Exception
 
-        except Exception as error:
+        if window("jellyfin_should_stop.bool"):
-            LOG.exception(error)
+            LOG.info("exiiiiitttinggg")
+            raise LibraryExitException("Should stop flag raised, exiting...")
 
-            raise LibraryException("StopCalled")
+        if not window("jellyfin_online.bool"):
+            raise LibraryExitException("Jellyfin not online, exiting...")
 
         return func(*args, **kwargs)
 
@@ -72,6 +78,7 @@ def stop(func):
 def jellyfin_item(func):
     """Wrapper to retrieve the jellyfin_db item."""
 
+    @wraps(func)
     def wrapper(self, item, *args, **kwargs):
         e_item = self.jellyfin_db.get_item_by_id(
             item["Id"] if isinstance(item, dict) else item

jellyfin_kodi/library.py

@@ -19,7 +19,11 @@ from .views import Views
 from .downloader import GetItemWorker
 from .helper import translate, api, stop, settings, window, dialog, event, LazyLogger
 from .helper.utils import split_list, set_screensaver, get_screensaver
-from .helper.exceptions import LibraryException
+from .helper.exceptions import (
+    LibraryException,
+    LibraryExitException,
+    LibrarySyncLaterException,
+)
 from .jellyfin import Jellyfin
 
 ##################################################################################################
@@ -93,7 +97,8 @@ class Library(threading.Thread):
 
             try:
                 self.service()
-            except LibraryException:
+            except LibraryException as error:
+                LOG.warning(error)
                 break
             except Exception as error:
                 LOG.exception(error)
@@ -438,11 +443,9 @@ class Library(threading.Thread):
                     return True
 
             return True
-        except LibraryException as error:
-            LOG.error(error.status)
-
-            if error.status in "SyncLibraryLater":
 
+        except LibrarySyncLaterException as error:
+            LOG.error(error)
             dialog("ok", "{jellyfin}", translate(33129))
             settings("SyncInstallRunDone.bool", True)
             sync = get_sync()
@@ -451,6 +454,9 @@ class Library(threading.Thread):
 
             return True
 
+        except LibraryException as error:
+            LOG.error(error)
+
         except Exception as error:
             LOG.exception(error)
 
@@ -758,8 +764,12 @@ class UpdateWorker(threading.Thread):
                             (item["Type"], api.API(item).get_naming())
                         )
                 except LibraryException as error:
-                    if error.status == "StopCalled":
+                    # TODO: Fixme; We're catching all LibraryException here,
+                    # but silently ignoring any that isn't the exit condition.
+                    # Investigate what would be appropriate behavior here.
+                    if isinstance(error, LibraryExitException):
                         break
+                    LOG.warning("Ignoring exception %s", error)
                 except Exception as error:
                     LOG.exception(error)
 
@@ -823,8 +833,12 @@ class UserDataWorker(threading.Thread):
                     elif item["Type"] == "Audio":
                         music.userdata(item)
                 except LibraryException as error:
-                    if error.status == "StopCalled":
+                    # TODO: Fixme; We're catching all LibraryException here,
+                    # but silently ignoring any that isn't the exit condition.
+                    # Investigate what would be appropriate behavior here.
+                    if isinstance(error, LibraryExitException):
                         break
+                    LOG.warning("Ignoring exception %s", error)
                 except Exception as error:
                     LOG.exception(error)
 
@@ -943,8 +957,12 @@ class RemovedWorker(threading.Thread):
                 try:
                     obj(item["Id"])
                 except LibraryException as error:
-                    if error.status == "StopCalled":
+                    # TODO: Fixme; We're catching all LibraryException here,
+                    # but silently ignoring any that isn't the exit condition.
+                    # Investigate what would be appropriate behavior here.
+                    if isinstance(error, LibraryExitException):
                         break
+                    LOG.warning("Ignoring exception %s", error)
                 except Exception as error:
                     LOG.exception(error)
                 finally:

mypy.ini

@@ -0,0 +1,4 @@
+[mypy]
+check_untyped_defs = True
+warn_unused_configs = True
+files = .

tox.ini

@@ -1,7 +1,7 @@
 [flake8]
 max-line-length = 9999
 import-order-style = pep8
-exclude = .git,.vscode
+exclude = .git,.vscode,.venv
 extend-ignore =
     I202
     E203