Improve network error handling and fix linter warnings (#370)

* Improve network error handling and fix linter warnings

* Raise errors happening in ThreadPoolExecutor, fix updates

blackboard_sync/__about__.py

@@ -16,7 +16,8 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with this program; if not, write to the Free Software
-# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+# MA  02110-1301, USA.
 
 __all__ = [
     "__id__",

blackboard_sync/__init__.py

@@ -42,6 +42,12 @@
     __copyright__
 )
 
+__all__ = [
+    '__id__', '__title__', '__summary__', '__uri__', '__homepage__',
+    '__author__', '__email__', '__publisher__', '__license__',
+    '__license_spdx__', '__copyright__'
+]
+
 # Console Output
 logger = logging.getLogger(__name__)
 logger.addHandler(logging.StreamHandler())

blackboard_sync/__main__.py

@@ -18,7 +18,8 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with this program; if not, write to the Free Software
-# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+# MA  02110-1301, USA.
 
 from .sync_controller import SyncController
 

blackboard_sync/config.py

@@ -16,11 +16,12 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with this program; if not, write to the Free Software
-# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+# MA  02110-1301, USA.
 
 import logging
 import configparser
-from typing import Any, Optional
+from typing import Any
 from pathlib import Path
 from functools import wraps
 from datetime import datetime
@@ -36,10 +37,14 @@
 class Config(configparser.ConfigParser):
     """Base configuration manager class, which wraps a ConfigParser."""
 
-
     def __init__(self, config_file: Path, *args, **kwargs):
-        super().__init__(converters={'path': Path, 'date': datetime.fromisoformat},
-                         interpolation=None, *args, **kwargs)
+        converters: dict[str, Callable[[str], Any]] = {
+            'path': Path, 'date': datetime.fromisoformat
+        }
+
+        super().__init__(converters=converters, interpolation=None,
+                         *args, **kwargs)
+
         self._config_file = config_file
         self.read(self._config_file)
 
@@ -68,28 +73,31 @@ class SyncConfig(Config):
     _config_filename = "blackboard_sync"
 
     def __init__(self, custom_dir=None):
-        config_dir = custom_dir or Path(user_config_dir(appauthor=__author__, roaming=True))
-        super().__init__(config_dir / self._config_filename, empty_lines_in_values=False)
+        default_dir = Path(user_config_dir(appauthor=__author__, roaming=True))
+
+        config_dir = custom_dir or default_dir
+        super().__init__(config_dir / self._config_filename,
+                         empty_lines_in_values=False)
 
         if 'Sync' not in self:
             self['Sync'] = {}
 
         self._sync = self['Sync']
 
     @property
-    def last_sync_time(self) -> Optional[datetime]:
+    def last_sync_time(self) -> datetime | None:
         return self._sync.getdate('last_sync_time')
 
     @last_sync_time.setter
     @Config.persist
-    def last_sync_time(self, last: Optional[datetime]) -> None:
+    def last_sync_time(self, last: datetime | None) -> None:
         if last is None:
             self.remove_option('Sync', 'last_sync_time')
         else:
             self._sync['last_sync_time'] = last.isoformat()
 
     @property
-    def download_location(self) -> Optional[Path]:
+    def download_location(self) -> Path | None:
         # Default download location
         default = Path(Path.home(), 'Downloads', 'BlackboardSync')
         return self._sync.getpath('download_location') or default
@@ -100,7 +108,7 @@ def download_location(self, sync_dir: Path) -> None:
         self._sync['download_location'] = str(sync_dir)
 
     @property
-    def university_index(self) -> Optional[int]:
+    def university_index(self) -> int | None:
         return self._sync.getint('university')
 
     @university_index.setter
@@ -109,10 +117,10 @@ def university_index(self, university: int) -> None:
         self._sync['university'] = str(university)
 
     @property
-    def min_year(self) -> Optional[int]:
+    def min_year(self) -> int | None:
         return self._sync.getint('min_year')
 
     @min_year.setter
     @Config.persist
-    def min_year(self, year: Optional[int])-> None:
+    def min_year(self, year: int | None) -> None:
         self._sync['min_year'] = str(year or 0)

blackboard_sync/content/content.py

@@ -24,7 +24,7 @@ class Content:
     """Content factory for all types."""
 
     def __init__(self, content: BBCourseContent, api_path: BBContentPath,
-                 job: DownloadJob):
+                 job: DownloadJob) -> None:
 
         logger.info(f"{content.title}[{content.contentHandler}]")
 
@@ -43,7 +43,7 @@ def __init__(self, content: BBCourseContent, api_path: BBContentPath,
         try:
             self.handler = Handler(content, api_path, job)
         except (ValidationError, JSONDecodeError,
-                BBBadRequestError, BBForbiddenError, RequestException):
+                BBBadRequestError, BBForbiddenError):
             logger.exception(f"Error fetching {content.title}")
 
         try:

blackboard_sync/download.py

@@ -19,23 +19,17 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with this program; if not, write to the Free Software
-# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+# MA  02110-1301, USA.
 
 import logging
-import platform
-from requests.exceptions import RequestException
 from pathlib import Path
-from typing import Optional
-from dateutil.parser import parse
 from datetime import datetime, timezone
-from concurrent.futures import ThreadPoolExecutor
 
 from blackboard.api_extended import BlackboardExtended
-from blackboard.blackboard import BBCourseContent, BBResourceType
 from blackboard.filters import BBMembershipFilter, BWFilter
 
-from .content import ExternalLink, ContentBody, Document, Folder, Content
-from .content import BBContentPath
+from .executor import SyncExecutor
 from .content.job import DownloadJob
 from .content.course import Course
 
@@ -50,8 +44,8 @@ class BlackboardDownload:
 
     def __init__(self, sess: BlackboardExtended,
                  download_location: Path,
-                 last_downloaded: Optional[datetime] = None,
-                 min_year: Optional[int] = None):
+                 last_downloaded: datetime | None = None,
+                 min_year: int | None = None):
         """BlackboardDownload constructor
 
         Download all files in blackboard recursively to download_location,
@@ -61,22 +55,21 @@ def __init__(self, sess: BlackboardExtended,
 
         :param BlackboardExtended sess: UCLan BB user session
         :param (str / Path) download_location: Where files will be stored
-        :param str last_downloaded: Files modified before this will not be downloaded
-        :param min_year: Only courses created on or after this year will be downloaded
+        :param str last_downloaded: Files modified before are ignored
+        :param min_year: Courses created before are ignored
         """
 
         self._sess = sess
         self._user_id = sess.user_id
         self._download_location = download_location
         self._min_year = min_year
-        self.executor = ThreadPoolExecutor(max_workers=8)
+        self.executor = SyncExecutor()
         self.cancelled = False
 
         if last_downloaded is not None:
             self._last_downloaded = last_downloaded
 
-
-    def download(self) -> Optional[datetime]:
+    def download(self) -> datetime | None:
         """Retrieve the user's courses, and start download of all contents
 
         :return: Datetime when method was called.
@@ -99,7 +92,8 @@ def download(self) -> Optional[datetime]:
         courses = self._sess.ex_fetch_courses(user_id=self.user_id,
                                               result_filter=course_filter)
 
-        job = DownloadJob(session=self._sess, last_downloaded=self._last_downloaded)
+        job = DownloadJob(session=self._sess,
+                          last_downloaded=self._last_downloaded)
 
         for course in courses:
             if self.cancelled:
@@ -109,8 +103,10 @@ def download(self) -> Optional[datetime]:
 
             Course(course, job).write(self.download_location, self.executor)
 
-        logger.info(f"Shutting down download workers")
+        logger.info("Shutting down download workers")
+
         self.executor.shutdown(wait=True, cancel_futures=self.cancelled)
+        self.executor.raise_exceptions()
 
         return start_time if not self.cancelled else None
 

blackboard_sync/executor.py

@@ -0,0 +1,44 @@
+# Copyright (C) 2024, Jacob Sánchez Pérez
+
+# This program 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 2
+# of the License, or (at your option) any later version.
+#
+# This program 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 this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+# MA  02110-1301, USA.
+
+from typing import Any
+from collections.abc import Callable
+
+from concurrent.futures import ThreadPoolExecutor, Future
+from concurrent.futures import wait as wait_futures
+
+
+class SyncExecutor(ThreadPoolExecutor):
+    def __init__(self, max_workers: int | None = None) -> None:
+        super().__init__(max_workers)
+        self.futures: list[Future[Any]] = []
+
+    def submit(self, fn: Callable[..., Any], /,
+               *args: Any, **kwargs: Any) -> Future[Any]:
+        future = super().submit(fn, *args, **kwargs)
+        self.futures.append(future)
+        return future
+
+    def shutdown(self, wait: bool = True, *,
+                 cancel_futures: bool = False) -> None:
+        super().shutdown(wait, cancel_futures=cancel_futures)
+
+    def raise_exceptions(self, timeout: int | None = None) -> None:
+        done, not_done = wait_futures(self.futures, timeout)
+
+        for future in done:
+            error = future.result()

blackboard_sync/institutions.py

@@ -14,13 +14,13 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with this program; if not, write to the Free Software
-# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+# MA  02110-1301, USA.
 
 import json
 import logging
 from pathlib import Path
 
-import requests
 from pydantic import HttpUrl, BaseModel
 
 from .ip import find_my_ip, find_ip_entity

blackboard_sync/ip.py

@@ -28,7 +28,7 @@ def find_my_ip() -> str | None:
     try:
         r = requests.get(IP_API, timeout=IP_TIMEOUT)
         r.raise_for_status()
-    except requests.RequestException: 
+    except requests.RequestException:
         return None
     else:
         return r.text

blackboard_sync/qt/dialogs.py

@@ -59,19 +59,6 @@ def redownload_dialog(self) -> bool:
         q.setWindowIcon(logo())
         return q.exec() == QMessageBox.StandardButton.Yes
 
-    def update_found_dialog(self) -> int:
-        q = QMessageBox()
-        q.setText(tr(
-            "A new version of BlackboardSync is now available!"
-        ))
-        q.setInformativeText(tr(
-            "Please download the latest version from your preferred store."
-        ))
-        q.setStandardButtons(QMessageBox.StandardButton.Ok)
-        q.setIcon(QMessageBox.Icon.Information)
-        q.setWindowIcon(logo())
-        return q.exec()
-
     def uni_not_supported_dialog(self, url: str) -> None:
         q = QMessageBox()
         q.setText(tr(

blackboard_sync/qt/manager.py

@@ -16,7 +16,6 @@
 # MA  02110-1301, USA.
 
 import sys
-import webbrowser
 from pathlib import Path
 from datetime import datetime
 from requests.cookies import RequestsCookieJar
@@ -69,7 +68,6 @@ def __init__(self, id: str, title: str, uri: str,
         else:
             self.app.setApplicationVersion(__version__)
 
-        self._has_shown_error = False
         self._init_ui(universities, autodetected)
 
     def _init_ui(self, universities: list[str],
@@ -219,7 +217,7 @@ def notify_login_error(self) -> None:
         self.show(self.login_window)
 
     def notify_sync_error(self) -> None:
-        if not self._has_shown_error:
-            self.tray.notify(Event.DOWNLOAD_ERROR)
-            webbrowser.open(self.help_uri)
-            self._has_shown_error = True
+        self.tray.notify(Event.DOWNLOAD_ERROR)
+
+    def notify_update(self) -> None:
+        self.tray.notify(Event.UPDATE_AVAILABLE)