[FEATURE] Process all subscriptions even if one or more error (#771)

Closes GH Issue #766, partially closes #520

Prior to this release, no other subscriptions would download if a subscription before it had an error. Now, subscriptions will continue to download even if one has an error, and will be reported in the output summary.

src/ytdl_sub/cli/entrypoint.py

@@ -5,7 +5,6 @@
 from pathlib import Path
 from typing import List
 from typing import Optional
-from typing import Tuple
 
 from yt_dlp.utils import sanitize_filename
 
@@ -20,7 +19,6 @@
 from ytdl_sub.utils.exceptions import ExperimentalFeatureNotEnabled
 from ytdl_sub.utils.exceptions import ValidationException
 from ytdl_sub.utils.file_handler import FileHandler
-from ytdl_sub.utils.file_handler import FileHandlerTransactionLog
 from ytdl_sub.utils.file_lock import working_directory_lock
 from ytdl_sub.utils.logger import Logger
 
@@ -31,6 +29,10 @@
 _VIEW_EXTRA_ARGS_FORMATTER = "--preset _view --overrides.url {}"
 
 
+def _log_time() -> str:
+    return datetime.now().strftime("%Y-%m-%d-%H%M%S")
+
+
 def _maybe_write_subscription_log_file(
     config: ConfigFile,
     subscription: Subscription,
@@ -51,23 +53,22 @@ def _maybe_write_subscription_log_file(
     if success and not config.config_options.persist_logs.keep_successful_logs:
         return
 
-    log_time = datetime.now().strftime("%Y-%m-%d-%H%M%S")
     log_subscription_name = sanitize_filename(subscription.name).lower().replace(" ", "_")
     log_success = "success" if success else "error"
 
-    log_filename = f"{log_time}.{log_subscription_name}.{log_success}.log"
+    log_filename = f"{_log_time()}.{log_subscription_name}.{log_success}.log"
     persist_log_path = Path(config.config_options.persist_logs.logs_directory) / log_filename
 
     if not success:
-        Logger.log_exit_exception(exception=exception, log_filepath=persist_log_path)
+        Logger.log_exception(exception=exception, log_filepath=persist_log_path)
 
     os.makedirs(os.path.dirname(persist_log_path), exist_ok=True)
     FileHandler.copy(Logger.debug_log_filename(), persist_log_path)
 
 
 def _download_subscriptions_from_yaml_files(
     config: ConfigFile, subscription_paths: List[str], update_with_info_json: bool, dry_run: bool
-) -> List[Tuple[Subscription, FileHandlerTransactionLog]]:
+) -> List[Subscription]:
     """
     Downloads all subscriptions from one or many subscription yaml files.
 
@@ -84,53 +85,49 @@ def _download_subscriptions_from_yaml_files(
 
     Returns
     -------
-    List of (subscription, transaction_log)
+    List of subscriptions processed
 
     Raises
     ------
     Exception
         Any exception during download
     """
     subscriptions: List[Subscription] = []
-    output: List[Tuple[Subscription, FileHandlerTransactionLog]] = []
 
     # Load all the subscriptions first to perform all validation before downloading
     for path in subscription_paths:
         subscriptions += Subscription.from_file_path(config=config, subscription_path=path)
 
     for subscription in subscriptions:
-        logger.info(
-            "Beginning subscription %s for %s",
-            ("dry run" if dry_run else "download"),
-            subscription.name,
-        )
-        logger.debug("Subscription full yaml:\n%s", subscription.as_yaml())
+        with subscription.exception_handling():
+            logger.info(
+                "Beginning subscription %s for %s",
+                ("dry run" if dry_run else "download"),
+                subscription.name,
+            )
+            logger.debug("Subscription full yaml:\n%s", subscription.as_yaml())
 
-        try:
             if update_with_info_json:
-                transaction_log = subscription.update_with_info_json(dry_run=dry_run)
+                subscription.update_with_info_json(dry_run=dry_run)
             else:
-                transaction_log = subscription.download(dry_run=dry_run)
-        except Exception as exc:  # pylint: disable=broad-except
-            _maybe_write_subscription_log_file(
-                config=config, subscription=subscription, dry_run=dry_run, exception=exc
-            )
-            raise
-        else:
-            output.append((subscription, transaction_log))
-            _maybe_write_subscription_log_file(
-                config=config, subscription=subscription, dry_run=dry_run
-            )
-            Logger.cleanup()  # Cleanup logger after each successful subscription download
-        finally:
-            gc.collect()  # Garbage collect after each subscription download
+                subscription.download(dry_run=dry_run)
 
-    return output
+        _maybe_write_subscription_log_file(
+            config=config,
+            subscription=subscription,
+            dry_run=dry_run,
+            exception=subscription.exception,
+        )
+
+        Logger.cleanup(cleanup_error_log=False)
+        gc.collect()  # Garbage collect after each subscription download
+
+    return subscriptions
 
 
 def _download_subscription_from_cli(
     config: ConfigFile, dry_run: bool, extra_args: List[str]
-) -> Tuple[Subscription, FileHandlerTransactionLog]:
+) -> Subscription:
     """
     Downloads a one-off subscription using the CLI
 
@@ -158,12 +155,12 @@ def _download_subscription_from_cli(
     )
 
     logger.info("Beginning CLI %s", ("dry run" if dry_run else "download"))
-    return subscription, subscription.download(dry_run=dry_run)
+    subscription.download(dry_run=dry_run)
 
+    return subscription
 
-def _view_url_from_cli(
-    config: ConfigFile, url: str, split_chapters: bool
-) -> Tuple[Subscription, FileHandlerTransactionLog]:
+
+def _view_url_from_cli(config: ConfigFile, url: str, split_chapters: bool) -> Subscription:
     """
     `ytdl-sub view` dry-runs a URL to print its source variables. Use the pre-built `_view` preset,
     inject the URL argument, and dry-run.
@@ -180,10 +177,12 @@ def _view_url_from_cli(
         url,
         " with split chapters" if split_chapters else "",
     )
-    return subscription, subscription.download(dry_run=True)
+    subscription.download(dry_run=True)
+
+    return subscription
 
 
-def main() -> List[Tuple[Subscription, FileHandlerTransactionLog]]:
+def main() -> List[Subscription]:
     """
     Entrypoint for ytdl-sub, without the error handling
     """
@@ -203,7 +202,7 @@ def main() -> List[Tuple[Subscription, FileHandlerTransactionLog]]:
         logger.info("No config specified, using defaults.")
         config = ConfigFile(name="default_config", value={})
 
-    transaction_logs: List[Tuple[Subscription, FileHandlerTransactionLog]] = []
+    subscriptions: List[Subscription] = []
 
     # If transaction log file is specified, make sure we can open it
     _maybe_validate_transaction_log_file(transaction_log_file_path=args.transaction_log)
@@ -222,7 +221,7 @@ def main() -> List[Tuple[Subscription, FileHandlerTransactionLog]]:
                     "full backup before usage. You have been warned!",
                 )
 
-            transaction_logs = _download_subscriptions_from_yaml_files(
+            subscriptions = _download_subscriptions_from_yaml_files(
                 config=config,
                 subscription_paths=args.subscription_paths,
                 update_with_info_json=args.update_with_info_json,
@@ -231,24 +230,24 @@ def main() -> List[Tuple[Subscription, FileHandlerTransactionLog]]:
 
         # One-off download
         elif args.subparser == "dl":
-            transaction_logs.append(
+            subscriptions.append(
                 _download_subscription_from_cli(
                     config=config, dry_run=args.dry_run, extra_args=extra_args
                 )
             )
         elif args.subparser == "view":
-            transaction_logs.append(
+            subscriptions.append(
                 _view_url_from_cli(config=config, url=args.url, split_chapters=args.split_chapters)
             )
         else:
             raise ValidationException("Must provide one of the commands: sub, dl, view")
 
     if not args.suppress_transaction_log:
         output_transaction_log(
-            transaction_logs=transaction_logs,
+            subscriptions=subscriptions,
             transaction_log_file_path=args.transaction_log,
         )
 
-    output_summary(transaction_logs)
+    output_summary(subscriptions)
 
-    return transaction_logs
+    return subscriptions

src/ytdl_sub/cli/output_summary.py

@@ -1,10 +1,8 @@
 from typing import List
-from typing import Tuple
 
 from colorama import Fore
 
 from ytdl_sub.subscriptions.subscription import Subscription
-from ytdl_sub.utils.file_handler import FileHandlerTransactionLog
 from ytdl_sub.utils.logger import Logger
 
 logger = Logger.get()
@@ -37,51 +35,52 @@ def _color_int(value: int) -> str:
     return _no_color(str_int)
 
 
-def output_summary(transaction_logs: List[Tuple[Subscription, FileHandlerTransactionLog]]) -> str:
+def output_summary(subscriptions: List[Subscription]) -> None:
     """
     Parameters
     ----------
-    transaction_logs
-        Transaction logs from downloaded subscriptions
+    subscriptions
+        Processed subscriptions
 
     Returns
     -------
     Output summary to print
     """
+    # many locals for proper output printing
+    # pylint: disable=too-many-locals
+    if len(subscriptions) == 0:
+        logger.info("No subscriptions ran")
+        return
+
     summary: List[str] = []
 
-    # Initialize widths to 0
-    width_sub_name: int = 0
-    width_num_entries_added: int = 0
-    width_num_entries_modified: int = 0
-    width_num_entries_removed: int = 0
-    width_num_entries: int = 0
-
-    # Calculate min width needed
-    for subscription, _ in transaction_logs:
-        width_sub_name = max(width_sub_name, len(subscription.name))
-        width_num_entries_added = max(
-            width_num_entries_added, len(_color_int(subscription.num_entries_added))
-        )
-        width_num_entries_modified = max(
-            width_num_entries_modified, len(_color_int(subscription.num_entries_modified))
-        )
-        width_num_entries_removed = max(
-            width_num_entries_removed, len(_color_int(subscription.num_entries_removed * -1))
-        )
-        width_num_entries = max(width_num_entries, len(str(subscription.num_entries)))
+    # Initialize totals to 0
+    total_subs: int = len(subscriptions)
+    total_subs_str = f"Total: {total_subs}"
+    total_added: int = sum(sub.num_entries_added for sub in subscriptions)
+    total_modified: int = sum(sub.num_entries_modified for sub in subscriptions)
+    total_removed: int = sum(sub.num_entries_removed for sub in subscriptions)
+    total_entries: int = sum(sub.num_entries for sub in subscriptions)
+    total_errors: int = sum(sub.exception is not None for sub in subscriptions)
 
-    # Add spacing for aesthetics
-    width_sub_name += 4
-    width_num_entries += 4
+    # Initialize widths to 0
+    width_sub_name: int = max(len(sub.name) for sub in subscriptions) + 4  # aesthetics
+    width_num_entries_added: int = len(_color_int(total_added))
+    width_num_entries_modified: int = len(_color_int(total_modified))
+    width_num_entries_removed: int = len(_color_int(total_removed))
+    width_num_entries: int = len(str(total_entries)) + 4  # aesthetics
 
     # Build the summary
-    for subscription, _ in transaction_logs:
+    for subscription in subscriptions:
         num_entries_added = _color_int(subscription.num_entries_added)
         num_entries_modified = _color_int(subscription.num_entries_modified)
         num_entries_removed = _color_int(subscription.num_entries_removed * -1)
         num_entries = str(subscription.num_entries)
-        status = _green("success")
+        status = (
+            _red(subscription.exception.__class__.__name__)
+            if subscription.exception
+            else _green("✔")
+        )
 
         summary.append(
             f"{subscription.name:<{width_sub_name}} "
@@ -92,5 +91,23 @@ def output_summary(transaction_logs: List[Tuple[Subscription, FileHandlerTransac
             f"{status}"
         )
 
+    total_errors_str = (
+        _green("Success") if total_errors == 0 else _red(f"Error{'s' if total_errors > 1 else ''}")
+    )
+
+    summary.append(
+        f"{total_subs_str:<{width_sub_name}} "
+        f"{_color_int(total_added):>{width_num_entries_added}} "
+        f"{_color_int(total_modified):>{width_num_entries_modified}} "
+        f"{_color_int(total_removed):>{width_num_entries_removed}} "
+        f"{total_entries:>{width_num_entries}} "
+        f"{total_errors_str}"
+    )
+
+    if total_errors > 0:
+        summary.append("")
+        summary.append(f"See `{Logger.error_log_filename()}` for details on errors.")
+        summary.append("Consider making a GitHub issue including the uploaded log file.")
+
     # Hack to always show download summary, even if logs are set to quiet
     logger.warning("Download Summary:\n%s", "\n".join(summary))

src/ytdl_sub/cli/output_transaction_log.py

@@ -1,10 +1,8 @@
 from typing import List
 from typing import Optional
-from typing import Tuple
 
 from ytdl_sub.subscriptions.subscription import Subscription
 from ytdl_sub.utils.exceptions import ValidationException
-from ytdl_sub.utils.file_handler import FileHandlerTransactionLog
 from ytdl_sub.utils.logger import Logger
 
 logger = Logger.get()
@@ -23,27 +21,27 @@ def _maybe_validate_transaction_log_file(transaction_log_file_path: Optional[str
 
 
 def output_transaction_log(
-    transaction_logs: List[Tuple[Subscription, FileHandlerTransactionLog]],
+    subscriptions: List[Subscription],
     transaction_log_file_path: Optional[str],
 ) -> None:
     """
     Maybe print and/or write transaction logs to a file
 
     Parameters
     ----------
-    transaction_logs
-        The transaction logs from downloaded subscriptions
+    subscriptions
+        Processed subscriptions
     transaction_log_file_path
         Optional file path to write to
     """
     transaction_log_file_contents = ""
-    for subscription, transaction_log in transaction_logs:
-        if transaction_log.is_empty:
+    for subscription in subscriptions:
+        if subscription.transaction_log.is_empty:
             transaction_log_contents = f"\nNo files changed for {subscription.name}"
         else:
             transaction_log_contents = (
                 f"Transaction log for {subscription.name}:\n"
-                f"{transaction_log.to_output_message(subscription.output_directory)}"
+                f"{subscription.transaction_log.to_output_message(subscription.output_directory)}"
             )
 
         if transaction_log_file_path: