🎨 Better separation of threads in importer

src/importer/importer.py

@@ -3,10 +3,11 @@ from pathlib import Path
 from threading import Lock, Thread
 
 import requests
+from requests import HTTPError
 from gi.repository import Adw, Gio, Gtk
 
 from .save_cover import resize_cover, save_cover
-from .steamgriddb import SGDBHelper
+from .steamgriddb import SGDBHelper, SGDBError
 
 
 class Importer:
@@ -85,7 +86,7 @@ class Importer:
             print(f"{source.full_name}, installed: {source.is_installed}")
             if not source.is_installed:
                 continue
-            thread = Thread(target=self.__import_source__, args=tuple([source]))  # fmt: skip
+            thread = SourceImportThread(self.win, source, self)
             self.source_threads.append(thread)
             thread.start()
 
@@ -107,83 +108,112 @@ class Importer:
 
         self.import_dialog.close()
 
-    def __import_source__(self, *args, **_kwargs):
+
-        """Source import thread entry point"""
+class SourceImportThread(Thread):
-        source, *_rest = args
+    """Thread in charge of scanning a source for games"""
+
+    win = None
+    source = None
+    importer = None
+
+    def __init__(self, win, source, importer, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.win = win
+        self.source = source
+        self.importer = importer
+
+    def run(self):
+        """Thread entry point"""
 
         # Initialize source iteration
-        iterator = source.__iter__()
+        iterator = iter(self.source)
-        with self.progress_lock:
+        with self.importer.progress_lock:
-            self.counts[source.id]["total"] = len(iterator)
+            self.importer.counts[self.source.id]["total"] = len(iterator)
-
-        # Handle iteration exceptions
-        def wrapper(iterator):
-            while True:
-                try:
-                    yield next(iterator)
-                except StopIteration:
-                    break
-                except Exception as exception:  # pylint: disable=broad-exception-caught
-                    logging.exception(
-                        msg=f"Exception in source {iterator.source.id}",
-                        exc_info=exception,
-                    )
-                    continue
 
         # Get games from source
-        for game in wrapper(iterator):
+        while True:
-            with self.games_lock:
+            # Handle exceptions raised while iteration the source
-                self.games.add(game)
+            try:
-            with self.progress_lock:
+                game = next(iterator)
-                self.counts[source.id]["games"] += 1
+            except StopIteration:
-            self.update_progressbar()
+                break
+            except Exception as exception:  # pylint: disable=broad-exception-caught
+                logging.exception(
+                    msg=f"Exception in source {self.source.id}",
+                    exc_info=exception,
+                )
+                continue
 
-            # Start sgdb lookup for game
+            # Add game to importer
+            with self.importer.games_lock:
+                self.importer.games.add(game)
+            with self.importer.progress_lock:
+                self.importer.counts[self.source.id]["games"] += 1
+            self.importer.update_progressbar()
+
+            # Start sgdb lookup for game in another thread
             # HACK move to a game manager
-            sgdb_thread = Thread(target=self.__sgdb_lookup__, args=tuple([game]))
+            # Skip obvious cases
-            with self.sgdb_threads_lock:
+            use_sgdb = self.win.schema.get_boolean("sgdb")
-                self.sgdb_threads.append(sgdb_thread)
+            if not use_sgdb or game.blacklisted:
+                return
+            sgdb_thread = SGDBLookupThread(self.win, game, self.importer)
+            with self.importer.sgdb_threads_lock:
+                self.importer.sgdb_threads.append(sgdb_thread)
             sgdb_thread.start()
 
-    def __sgdb_lookup__(self, *args, **_kwargs):
-        """SGDB lookup thread entry point"""
-        game, *_rest = args
 
-        def inner():
+class SGDBLookupThread(Thread):
-            # Skip obvious ones
+    """Thread in charge of querying SGDB for a game image"""
-            if game.blacklisted:
+
-                return
+    win = None
-            use_sgdb = self.win.schema.get_boolean("sgdb")
+    game = None
-            if not use_sgdb:
+    importer = None
-                return
+
-            # Check if we should query SGDB
+    def __init__(self, win, game, importer, *args, **kwargs):
-            prefer_sgdb = self.win.schema.get_boolean("sgdb-prefer")
+        super().__init__(*args, **kwargs)
-            prefer_animated = self.win.schema.get_boolean("sgdb-animated")
+        self.win = win
-            image_trunk = self.win.covers_dir / game.game_id
+        self.game = game
-            still = image_trunk.with_suffix(".tiff")
+        self.importer = importer
-            animated = image_trunk.with_suffix(".gif")
+
-            # breaking down the condition
+    def run(self):
-            is_missing = not still.is_file() and not animated.is_file()
+        """Thread entry point"""
-            is_not_best = not animated.is_file() and prefer_animated
+
-            should_query = is_missing or is_not_best or prefer_sgdb
+        # Check if we should query SGDB
-            if not should_query:
+        prefer_sgdb = self.win.schema.get_boolean("sgdb-prefer")
-                return
+        prefer_animated = self.win.schema.get_boolean("sgdb-animated")
-            # Add image from sgdb
+        image_trunk = self.win.covers_dir / self.game.game_id
-            game.set_loading(1)
+        still = image_trunk.with_suffix(".tiff")
-            sgdb = SGDBHelper(self.win)
+        animated = image_trunk.with_suffix(".gif")
-            uri = sgdb.get_game_image_uri(game, animated=prefer_animated)
+
+        # Breaking down the condition
+        is_missing = not still.is_file() and not animated.is_file()
+        is_not_best = not animated.is_file() and prefer_animated
+        if not (is_missing or is_not_best or prefer_sgdb):
+            return
+
+        self.game.set_loading(1)
+
+        # Add image from sgdb
+        sgdb = SGDBHelper(self.win)
+        try:
+            sgdb_id = sgdb.get_game_id(self.game)
+            uri = sgdb.get_game_image_uri(sgdb_id, animated=prefer_animated)
             response = requests.get(uri, timeout=5)
+        except HTTPError as _error:
+            # TODO handle http errors
+            pass
+        except SGDBError as _error:
+            # TODO handle SGDB API errors
+            pass
+        else:
             tmp_file = Gio.File.new_tmp()[0]
             tmp_file_path = tmp_file.get_path()
             Path(tmp_file_path).write_bytes(response.content)
-            save_cover(self.win, game.game_id, resize_cover(self.win, tmp_file_path))
+            save_cover(
-            game.set_loading(0)
+                self.win, self.game.game_id, resize_cover(self.win, tmp_file_path)
+            )
 
-        try:
+        self.game.set_loading(0)
-            inner()
+        with self.importer.progress_lock:
-        except Exception:  # pylint: disable=broad-exception-caught
+            self.importer.counts[self.game.source.id]["covers"] += 1
-            # TODO for god's sake handle exceptions correctly
-            # TODO (talk about that with Kramo)
-            pass
-        with self.progress_lock:
-            self.counts[game.source]["covers"] += 1

src/utils/steamgriddb.py

@@ -64,9 +64,8 @@ class SGDBHelper:
             raise SGDBError(res_json["errors"])
         raise SGDBError(res.status_code)
 
-    def get_game_image_uri(self, game, animated=False):
+    def get_image_uri(self, game_id, animated=False):
-        """Get the image for a game"""
+        """Get the image for a SGDB game id"""
-        game_id = self.get_game_id(game)
         uri = f"{self.base_url}grids/game/{game_id}?dimensions=600x900"
         if animated:
             uri += "&types=animated"