From 98f02da36c5452f50c0988b11842582b84c5642b Mon Sep 17 00:00:00 2001 From: GeoffreyCoulaud Date: Wed, 7 Jun 2023 14:01:06 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=8E=A8=20SourceIterator=20can=20yield=20a?= =?UTF-8?q?ddtitional=20data?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SourceIterator-s can yield a game and a tuple of additional data. This data will be passed to the Store, Pipeline and Managers. --- src/importer/importer.py | 28 +++++++++++++++++++------ src/importer/sources/itch_source.py | 9 +++----- src/importer/sources/source.py | 10 +++++---- src/main.py | 2 +- src/store/managers/async_manager.py | 10 +++++---- src/store/managers/display_manager.py | 2 +- src/store/managers/file_manager.py | 2 +- src/store/managers/manager.py | 20 +++++++++++------- src/store/managers/sgdb_manager.py | 6 ++++-- src/store/managers/steam_api_manager.py | 2 +- src/store/pipeline.py | 8 +++++-- src/store/store.py | 6 ++++-- 12 files changed, 68 insertions(+), 37 deletions(-) diff --git a/src/importer/importer.py b/src/importer/importer.py index 92e41f1..bd463e1 100644 --- a/src/importer/importer.py +++ b/src/importer/importer.py @@ -3,9 +3,9 @@ import logging from gi.repository import Adw, Gio, Gtk from src import shared +from src.game import Game from src.utils.task import Task from src.store.pipeline import Pipeline -from src.store.managers.manager import Manager from src.importer.sources.source import Source @@ -92,6 +92,7 @@ class Importer: def source_task_thread_func(self, _task, _obj, data, _cancellable): """Source import task code""" + source: Source source, *_rest = data # Early exit if not installed @@ -107,20 +108,35 @@ class Importer: while True: # Handle exceptions raised when iterating try: - game = next(iterator) + iteration_result = next(iterator) except StopIteration: break except Exception as exception: # pylint: disable=broad-exception-caught logging.exception( - msg=f"Exception in source {source.id}", - exc_info=exception, + "Exception in source %s", source.id, exc_info=exception ) continue - if game is None: + + # Handle the result depending on its type + if isinstance(iteration_result, Game): + game = iteration_result + additional_data = tuple() + elif isinstance(iteration_result, tuple): + game, additional_data = iteration_result + elif iteration_result is None: + continue + else: + # Warn source implementers that an invalid type was produced + # Should not happen on production code + logging.warn( + "%s produced an invalid iteration return type %s", + source.id, + type(iteration_result), + ) continue # Register game - pipeline: Pipeline = shared.store.add_game(game) + pipeline: Pipeline = shared.store.add_game(game, additional_data) if pipeline is not None: logging.info("Imported %s (%s)", game.name, game.game_id) pipeline.connect("advanced", self.pipeline_advanced_callback) diff --git a/src/importer/sources/itch_source.py b/src/importer/sources/itch_source.py index 1b90ffc..eddbb2e 100644 --- a/src/importer/sources/itch_source.py +++ b/src/importer/sources/itch_source.py @@ -48,12 +48,9 @@ class ItchSourceIterator(SourceIterator): "game_id": self.source.game_id_format.format(game_id=row[0]), "executable": self.source.executable_format.format(cave_id=row[4]), } - yield Game(values, allow_side_effects=False) - - # TODO pass image URIs to the pipeline somehow - # - Add a reserved field to the Game object - # - Reconstruct those from the pipeline (we already have them) - # - Pass game and additional data to the pipeline separately (requires deep changes) + additional_data = (row[3], row[2]) + game = Game(values, allow_side_effects=False) + yield (game, additional_data) class ItchSource(Source): diff --git a/src/importer/sources/source.py b/src/importer/sources/source.py index 904368b..8fa4283 100644 --- a/src/importer/sources/source.py +++ b/src/importer/sources/source.py @@ -3,12 +3,15 @@ from abc import abstractmethod from collections.abc import Iterable, Iterator from functools import wraps from pathlib import Path -from typing import Generator, Optional +from typing import Generator, Any from src import shared from src.game import Game from src.utils.decorators import replaced_by_path +# Type of the data returned by iterating on a Source +SourceIterationResult = None | Game | tuple[Game, tuple[Any]] + class SourceIterator(Iterator): """Data producer for a source of games""" @@ -24,11 +27,11 @@ class SourceIterator(Iterator): def __iter__(self) -> "SourceIterator": return self - def __next__(self) -> Optional[Game]: + def __next__(self) -> SourceIterationResult: return next(self.generator) @abstractmethod - def generator_builder(self) -> Generator[Optional[Game], None, None]: + def generator_builder(self) -> Generator[SourceIterationResult, None, None]: """ Method that returns a generator that produces games * Should be implemented as a generator method @@ -49,7 +52,6 @@ class Source(Iterable): def __init__(self) -> None: super().__init__() self.available_on = set() - @property def full_name(self) -> str: diff --git a/src/main.py b/src/main.py index d379d10..1c05625 100644 --- a/src/main.py +++ b/src/main.py @@ -133,7 +133,7 @@ class CartridgesApplication(Adw.Application): for game_file in shared.games_dir.iterdir(): data = json.load(game_file.open()) game = Game(data, allow_side_effects=False) - shared.store.add_game(game) + shared.store.add_game(game, tuple()) def on_about_action(self, *_args): about = Adw.AboutWindow( diff --git a/src/store/managers/async_manager.py b/src/store/managers/async_manager.py index 8d54681..a69f161 100644 --- a/src/store/managers/async_manager.py +++ b/src/store/managers/async_manager.py @@ -26,16 +26,18 @@ class AsyncManager(Manager): Already scheduled Tasks will no longer be cancellable.""" self.cancellable = Gio.Cancellable() - def process_game(self, game: Game, callback: Callable[["Manager"], Any]) -> None: + def process_game( + self, game: Game, additional_data: tuple, callback: Callable[["Manager"], Any] + ) -> None: """Create a task to process the game in a separate thread""" task = Task.new(None, self.cancellable, self._task_callback, (callback,)) - task.set_task_data((game,)) + task.set_task_data((game, additional_data)) task.run_in_thread(self._task_thread_func) def _task_thread_func(self, _task, _source_object, data, cancellable): """Task thread entry point""" - game, *_rest = data - self.execute_resilient_manager_logic(game) + game, additional_data, *_rest = data + self.execute_resilient_manager_logic(game, additional_data) def _task_callback(self, _source_object, _result, data): """Method run after the task is done""" diff --git a/src/store/managers/display_manager.py b/src/store/managers/display_manager.py index 90a7bde..cc0d82d 100644 --- a/src/store/managers/display_manager.py +++ b/src/store/managers/display_manager.py @@ -10,7 +10,7 @@ class DisplayManager(Manager): run_after = set((SteamAPIManager, SGDBManager)) - def manager_logic(self, game: Game) -> None: + def manager_logic(self, game: Game, _additional_data: tuple) -> None: # TODO decouple a game from its widget shared.win.games[game.game_id] = game game.update() diff --git a/src/store/managers/file_manager.py b/src/store/managers/file_manager.py index 07c725f..69901ee 100644 --- a/src/store/managers/file_manager.py +++ b/src/store/managers/file_manager.py @@ -8,5 +8,5 @@ class FileManager(AsyncManager): run_after = set((SteamAPIManager,)) - def manager_logic(self, game: Game) -> None: + def manager_logic(self, game: Game, _additional_data: tuple) -> None: game.save() diff --git a/src/store/managers/manager.py b/src/store/managers/manager.py index 9675aeb..5b1b7d6 100644 --- a/src/store/managers/manager.py +++ b/src/store/managers/manager.py @@ -37,7 +37,7 @@ class Manager: self.errors_lock = Lock() def report_error(self, error: Exception): - """Report an error that happened in Manager.run""" + """Report an error that happened in Manager.process_game""" with self.errors_lock: self.errors.append(error) @@ -49,7 +49,7 @@ class Manager: return errors @abstractmethod - def manager_logic(self, game: Game) -> None: + def manager_logic(self, game: Game, additional_data: tuple) -> None: """ Manager specific logic triggered by the run method * Implemented by final child classes @@ -58,10 +58,12 @@ class Manager: * May raise other exceptions that will be reported """ - def execute_resilient_manager_logic(self, game: Game, try_index: int = 0) -> None: + def execute_resilient_manager_logic( + self, game: Game, additional_data: tuple, try_index: int = 0 + ) -> None: """Execute the manager logic and handle its errors by reporting them or retrying""" try: - self.manager_logic(game) + self.manager_logic(game, additional_data) except Exception as error: if error in self.continue_on: # Handle skippable errors (skip silently) @@ -71,7 +73,9 @@ class Manager: # Handle retryable errors logging_format = "Retrying %s in %s for %s" sleep(self.retry_delay) - self.execute_resilient_manager_logic(game, try_index + 1) + self.execute_resilient_manager_logic( + game, additional_data, try_index + 1 + ) else: # Handle being out of retries logging_format = "Out of retries dues to %s in %s for %s" @@ -88,7 +92,9 @@ class Manager: f"{game.name} ({game.game_id})", ) - def process_game(self, game: Game, callback: Callable[["Manager"], Any]) -> None: + def process_game( + self, game: Game, additional_data: tuple, callback: Callable[["Manager"], Any] + ) -> None: """Pass the game through the manager""" - self.execute_resilient_manager_logic(game) + self.execute_resilient_manager_logic(game, additional_data) callback(self) diff --git a/src/store/managers/sgdb_manager.py b/src/store/managers/sgdb_manager.py index 2179ee9..5b902df 100644 --- a/src/store/managers/sgdb_manager.py +++ b/src/store/managers/sgdb_manager.py @@ -1,3 +1,5 @@ +from urllib3.exceptions import SSLError + from src.game import Game from src.store.managers.async_manager import AsyncManager from src.store.managers.steam_api_manager import SteamAPIManager @@ -8,9 +10,9 @@ class SGDBManager(AsyncManager): """Manager in charge of downloading a game's cover from steamgriddb""" run_after = set((SteamAPIManager,)) - retryable_on = set((HTTPError,)) + retryable_on = set((HTTPError, SSLError)) - def manager_logic(self, game: Game) -> None: + def manager_logic(self, game: Game, _additional_data: tuple) -> None: try: sgdb = SGDBHelper() sgdb.conditionaly_update_cover(game) diff --git a/src/store/managers/steam_api_manager.py b/src/store/managers/steam_api_manager.py index 5874735..500a358 100644 --- a/src/store/managers/steam_api_manager.py +++ b/src/store/managers/steam_api_manager.py @@ -15,7 +15,7 @@ class SteamAPIManager(AsyncManager): retryable_on = set((HTTPError, SSLError)) - def manager_logic(self, game: Game) -> None: + def manager_logic(self, game: Game, _additional_data: tuple) -> None: # Skip non-steam games if not game.source.startswith("steam_"): return diff --git a/src/store/pipeline.py b/src/store/pipeline.py index 63fc7d3..a13253c 100644 --- a/src/store/pipeline.py +++ b/src/store/pipeline.py @@ -11,14 +11,18 @@ class Pipeline(GObject.Object): """Class representing a set of managers for a game""" game: Game + additional_data: tuple waiting: set[Manager] running: set[Manager] done: set[Manager] - def __init__(self, game: Game, managers: Iterable[Manager]) -> None: + def __init__( + self, game: Game, additional_data: tuple, managers: Iterable[Manager] + ) -> None: super().__init__() self.game = game + self.additional_data = additional_data self.waiting = set(managers) self.running = set() self.done = set() @@ -72,7 +76,7 @@ class Pipeline(GObject.Object): for manager in (*parallel, *blocking): self.waiting.remove(manager) self.running.add(manager) - manager.process_game(self.game, self.manager_callback) + manager.process_game(self.game, self.additional_data, self.manager_callback) def manager_callback(self, manager: Manager) -> None: """Method called by a manager when it's done""" diff --git a/src/store/store.py b/src/store/store.py index 4c260c9..5ecd79a 100644 --- a/src/store/store.py +++ b/src/store/store.py @@ -20,7 +20,9 @@ class Store: """Add a manager class that will run when games are added""" self.managers.add(manager) - def add_game(self, game: Game, replace=False) -> Pipeline | None: + def add_game( + self, game: Game, additional_data: tuple, replace=False + ) -> Pipeline | None: """Add a game to the app if not already there :param replace bool: Replace the game if it already exists @@ -49,7 +51,7 @@ class Store: return None # Run the pipeline for the game - pipeline = Pipeline(game, self.managers) + pipeline = Pipeline(game, additional_data, self.managers) self.games[game.game_id] = game self.pipelines[game.game_id] = pipeline pipeline.advance()