From ad1ab33442cad315b971ec980d532e4139e938a0 Mon Sep 17 00:00:00 2001 From: Jesse Bannon Date: Tue, 7 Mar 2023 16:50:58 -0800 Subject: [PATCH] logging refactored with persistence --- src/ytdl_sub/cli/main.py | 42 +++++++++++++++---------- src/ytdl_sub/config/config_validator.py | 17 +++++++--- src/ytdl_sub/main.py | 8 +---- src/ytdl_sub/utils/logger.py | 31 +++++++++++------- tests/unit/utils/test_logger.py | 2 +- 5 files changed, 58 insertions(+), 42 deletions(-) diff --git a/src/ytdl_sub/cli/main.py b/src/ytdl_sub/cli/main.py index 826c8538..a0dfaa50 100644 --- a/src/ytdl_sub/cli/main.py +++ b/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 diff --git a/src/ytdl_sub/config/config_validator.py b/src/ytdl_sub/config/config_validator.py index b35c8b84..4b029efb 100644 --- a/src/ytdl_sub/config/config_validator.py +++ b/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) diff --git a/src/ytdl_sub/main.py b/src/ytdl_sub/main.py index b3a27669..6a5fd38c 100644 --- a/src/ytdl_sub/main.py +++ b/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) diff --git a/src/ytdl_sub/utils/logger.py b/src/ytdl_sub/utils/logger.py index bcfb2891..ba686d75 100644 --- a/src/ytdl_sub/utils/logger.py +++ b/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 diff --git a/tests/unit/utils/test_logger.py b/tests/unit/utils/test_logger.py index 9509b546..9cfdb092 100644 --- a/tests/unit/utils/test_logger.py +++ b/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(