logging refactored with persistence

src/ytdl_sub/cli/main.py

@@ -1,9 +1,10 @@
 import argparse
-from datetime import datetime
 import gc
 import sys
+from datetime import datetime
 from pathlib import Path
 from typing import List
+from typing import Optional
 from typing import Tuple
 
 from yt_dlp.utils import sanitize_filename
@@ -12,7 +13,8 @@ from ytdl_sub.cli.download_args_parser import DownloadArgsParser
 from ytdl_sub.cli.main_args_parser import parser
 from ytdl_sub.config.config_file import ConfigFile
 from ytdl_sub.subscriptions.subscription import Subscription
-from ytdl_sub.utils.file_handler import FileHandlerTransactionLog, FileHandler
+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
 
@@ -22,14 +24,17 @@ logger = Logger.get()
 # Use ytdl-sub dl arguments to use the preset
 _VIEW_EXTRA_ARGS_FORMATTER = "--preset _view --overrides.url {}"
 
-def _maybe_write_subscription_log_file(config: ConfigFile, subscription: Subscription, success: bool) -> None:
+
+def _maybe_write_subscription_log_file(
+    config: ConfigFile, subscription: Subscription, success: bool
+) -> Optional[Path]:
     # If persisting logs is disabled, do nothing
     if not config.config_options.persist_logs:
-        return
+        return None
 
     # If persisting successful logs is disabled, do nothing
     if success and not config.config_options.persist_logs.keep_successful_logs:
-        return
+        return None
 
     log_time = datetime.now().strftime("%Y-%m-%d-%H%M%S")
     log_subscription_name = sanitize_filename(subscription.name).lower().replace(" ", "_")
@@ -39,6 +44,7 @@ def _maybe_write_subscription_log_file(config: ConfigFile, subscription: Subscri
     persist_log_path = Path(config.config_options.persist_logs.logs_directory) / log_filename
 
     FileHandler.copy(Logger.debug_log_filename(), persist_log_path)
+    return persist_log_path
 
 
 def _download_subscriptions_from_yaml_files(
@@ -60,7 +66,8 @@ def _download_subscriptions_from_yaml_files(
 
     Raises
     ------
-    Validation exception if main arg is specified as a subscription path
+    Exception
+        Any exception during download
     """
     subscription_paths: List[str] = args.subscription_paths
     subscriptions: List[Subscription] = []
@@ -80,19 +87,20 @@ def _download_subscriptions_from_yaml_files(
 
         try:
             transaction_log = subscription.download(dry_run=args.dry_run)
-        except Exception as exc:
-            Logger.log_exit_exception(logger=logger, exception=exc)
-            _maybe_write_subscription_log_file(
-                config=config, subscription=subscription,
-                success=False
+        except Exception as exc:  # pylint: disable=broad-except
+            persisted_log_path = _maybe_write_subscription_log_file(
+                config=config, subscription=subscription, success=False
             )
+            Logger.log_exit_exception(exception=exc, log_filepath=persisted_log_path)
             raise
-
-        output.append((subscription, transaction_log))
-        gc.collect()  # Garbage collect after each subscription download
-
-        _maybe_write_subscription_log_file(config=config, subscription=subscription, success=True)
-        Logger.cleanup()  # Cleanup logger after each successful subscription download
+        else:
+            output.append((subscription, transaction_log))
+            _maybe_write_subscription_log_file(
+                config=config, subscription=subscription, success=True
+            )
+            Logger.cleanup()  # Cleanup logger after each successful subscription download
+        finally:
+            gc.collect()  # Garbage collect after each subscription download
 
     return output
 

src/ytdl_sub/config/config_validator.py

@@ -10,7 +10,8 @@ from ytdl_sub.utils.system import IS_WINDOWS
 from ytdl_sub.validators.file_path_validators import FFmpegFileValidator
 from ytdl_sub.validators.file_path_validators import FFprobeFileValidator
 from ytdl_sub.validators.strict_dict_validator import StrictDictValidator
-from ytdl_sub.validators.validators import LiteralDictValidator, BoolValidator
+from ytdl_sub.validators.validators import BoolValidator
+from ytdl_sub.validators.validators import LiteralDictValidator
 from ytdl_sub.validators.validators import StringValidator
 
 if IS_WINDOWS:
@@ -22,6 +23,7 @@ else:
     _DEFAULT_FFMPEG_PATH = "/usr/bin/ffmpeg"
     _DEFAULT_FFPROBE_PATH = "/usr/bin/ffprobe"
 
+
 class PersistLogsValidator(StrictDictValidator):
     _required_keys = {"logs_directory"}
     _optional_keys = {"keep_logs_after", "keep_successful_logs"}
@@ -29,9 +31,7 @@ class PersistLogsValidator(StrictDictValidator):
     def __init__(self, name: str, value: Any):
         super().__init__(name, value)
 
-        self._logs_directory = self._validate_key(
-            key="logs_directory", validator=StringValidator
-        )
+        self._logs_directory = self._validate_key(key="logs_directory", validator=StringValidator)
 
         self._keep_logs_after: Optional[str] = None
         if keep_logs_validator := self._validate_key_if_present(
@@ -61,7 +61,14 @@ class PersistLogsValidator(StrictDictValidator):
 
 class ConfigOptions(StrictDictValidator):
     _required_keys = {"working_directory"}
-    _optional_keys = {"umask", "dl_aliases", "persist_logs", "lock_directory", "ffmpeg_path", "ffprobe_path"}
+    _optional_keys = {
+        "umask",
+        "dl_aliases",
+        "persist_logs",
+        "lock_directory",
+        "ffmpeg_path",
+        "ffprobe_path",
+    }
 
     def __init__(self, name: str, value: Any):
         super().__init__(name, value)

src/ytdl_sub/main.py

@@ -1,8 +1,6 @@
 import sys
 
-from ytdl_sub import __local_version__
 from ytdl_sub.cli.main_args_parser import parser
-from ytdl_sub.utils.exceptions import ValidationException
 from ytdl_sub.utils.logger import Logger
 
 
@@ -24,15 +22,11 @@ def main():
     """
     Entrypoint for ytdl-sub
     """
-    logger = Logger.get()
     try:
         _main()
         Logger.cleanup()  # Ran successfully, so we can delete the debug file
     except Exception as exc:  # pylint: disable=broad-except
-        Logger.log_exit_exception(
-            logger=logger,
-            exception=exc,
-        )
+        Logger.log_exit_exception(exception=exc)
         sys.exit(1)
 
     sys.exit(0)

src/ytdl_sub/utils/logger.py

@@ -4,6 +4,7 @@ import logging
 import sys
 import tempfile
 from dataclasses import dataclass
+from pathlib import Path
 from typing import List
 from typing import Optional
 
@@ -200,8 +201,20 @@ class Logger:
                     yield
 
     @classmethod
-    def log_exit_exception(cls, logger: logging.Logger, exception: Exception, filename: Optional[str] = None):
+    def log_exit_exception(cls, exception: Exception, log_filepath: Optional[Path] = None):
+        """
+        Performs the final log before exiting from an error
+
+        Parameters
+        ----------
+        exception
+            The exception to log
+        log_filepath
+            Optional. The filepath to the debug logs
+        """
         if not cls._LOGGED_EXIT_EXCEPTION:
+            logger = cls.get()
+
             # Log validation exceptions as-is
             if isinstance(exception, ValidationException):
                 logger.error(exception)
@@ -213,20 +226,15 @@ class Logger:
                     "issue at https://github.com/jmbannon/ytdl-sub/issues with your config and "
                     "command/subscription yaml file to reproduce. Thanks for trying ytdl-sub!",
                     __local_version__,
-                    filename if filename else Logger.debug_log_filename(),
+                    log_filepath if log_filepath else Logger.debug_log_filename(),
                 )
 
             cls._LOGGED_EXIT_EXCEPTION = True
 
     @classmethod
-    def cleanup(cls, delete_debug_file: bool = True):
+    def cleanup(cls):
         """
-        Cleans up any log files left behind
-
-        Parameters
-        ----------
-        delete_debug_file
-            Whether to delete the debug log file. Defaults to True.
+        Cleans up debug log file left behind
         """
         for logger in cls._LOGGERS:
             for handler in logger.handlers:
@@ -234,6 +242,5 @@ class Logger:
 
         cls._DEBUG_LOGGER_FILE.close()
 
-        if delete_debug_file:
-            FileHandler.delete(cls.debug_log_filename())
-            cls._LOGGED_EXIT_EXCEPTION = False
+        FileHandler.delete(cls.debug_log_filename())
+        cls._LOGGED_EXIT_EXCEPTION = False

tests/unit/utils/test_logger.py

@@ -114,7 +114,7 @@ class TestLogger:
                 "[ytdl-sub:name_test] debug test\n",
             ]
 
-            Logger.cleanup(delete_debug_file=True)
+            Logger.cleanup()
             assert not os.path.isfile(Logger._DEBUG_LOGGER_FILE.name)
 
     @pytest.mark.parametrize(