From fddf3d3088e684f5b6f9e211146dee37f84f0399 Mon Sep 17 00:00:00 2001 From: Jesse Bannon Date: Wed, 8 Mar 2023 13:32:07 -0800 Subject: [PATCH] [FEATURE] Ability to suppress or write transaction log to file (#514) --- docs/usage.rst | 10 +- src/ytdl_sub/cli/main.py | 54 +++++- src/ytdl_sub/cli/main_args_parser.py | 35 +++- tests/unit/cli/test_main.py | 261 +++++++++++++++++++-------- 4 files changed, 273 insertions(+), 87 deletions(-) diff --git a/docs/usage.rst b/docs/usage.rst index cc42640..b0d01cb 100644 --- a/docs/usage.rst +++ b/docs/usage.rst @@ -15,12 +15,16 @@ General options must be specified before the command (i.e. ``sub``). .. code-block:: text -h, --help show this help message and exit + -v, --version show program's version number and exit -c CONFIGPATH, --config CONFIGPATH path to the config yaml, uses config.yaml if not provided - --dry-run preview what a download would output, does not perform any - video downloads or writes to output directories - --log-level quiet|info|verbose|debug + -d, --dry-run preview what a download would output, does not perform any video downloads or writes to output directories + -l quiet|info|verbose|debug, --log-level quiet|info|verbose|debug level of logs to print to console, defaults to info + -t TRANSACTIONPATH, --transaction-log TRANSACTIONPATH + path to store the transaction log output of all files added, modified, deleted + -st, --suppress-transaction-log + do not output transaction logs to console or file Sub Options ----------- diff --git a/src/ytdl_sub/cli/main.py b/src/ytdl_sub/cli/main.py index 6e798ad..b5a1fa6 100644 --- a/src/ytdl_sub/cli/main.py +++ b/src/ytdl_sub/cli/main.py @@ -13,6 +13,7 @@ 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.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 @@ -172,6 +173,42 @@ def _view_url_from_cli( return subscription, subscription.download(dry_run=True) +def _maybe_validate_transaction_log_file(transaction_log_file_path: Optional[str]) -> None: + if transaction_log_file_path: + try: + with open(transaction_log_file_path, "w", encoding="utf-8"): + pass + except Exception as exc: + raise ValidationException( + f"Transaction log file '{transaction_log_file_path}' cannot be written to. " + f"Reason: {str(exc)}" + ) from exc + + +def _output_transaction_log( + transaction_logs: List[Tuple[Subscription, FileHandlerTransactionLog]], + transaction_log_file_path: str, +) -> None: + transaction_log_file_contents = "" + for subscription, transaction_log in transaction_logs: + if transaction_log.is_empty: + transaction_log_contents = f"No 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)}" + ) + + if transaction_log_file_path: + transaction_log_file_contents += transaction_log_contents + else: + logger.info(transaction_log_contents) + + if transaction_log_file_contents: + with open(transaction_log_file_path, "w", encoding="utf-8") as transaction_log_file: + transaction_log_file.write(transaction_log_file_contents) + + def main() -> List[Tuple[Subscription, FileHandlerTransactionLog]]: """ Entrypoint for ytdl-sub, without the error handling @@ -187,6 +224,9 @@ def main() -> List[Tuple[Subscription, FileHandlerTransactionLog]]: config: ConfigFile = ConfigFile.from_file_path(args.config) transaction_logs: List[Tuple[Subscription, FileHandlerTransactionLog]] = [] + # If transaction log file is specified, make sure we can open it + _maybe_validate_transaction_log_file(transaction_log_file_path=args.transaction_log) + with working_directory_lock(config=config): if args.subparser == "sub": transaction_logs = _download_subscriptions_from_yaml_files( @@ -207,14 +247,10 @@ def main() -> List[Tuple[Subscription, FileHandlerTransactionLog]]: _view_url_from_cli(config=config, url=args.url, split_chapters=args.split_chapters) ) - for subscription, transaction_log in transaction_logs: - if transaction_log.is_empty: - logger.info("No files changed for %s", subscription.name) - else: - logger.info( - "Downloads for %s:\n%s\n", - subscription.name, - transaction_log.to_output_message(subscription.output_directory), - ) + if not args.suppress_transaction_log: + _output_transaction_log( + transaction_logs=transaction_logs, + transaction_log_file_path=args.transaction_log, + ) return transaction_logs diff --git a/src/ytdl_sub/cli/main_args_parser.py b/src/ytdl_sub/cli/main_args_parser.py index 220fb1b..e5879f9 100644 --- a/src/ytdl_sub/cli/main_args_parser.py +++ b/src/ytdl_sub/cli/main_args_parser.py @@ -28,6 +28,16 @@ class MainArguments: long="--log-level", is_positional=True, ) + TRANSACTION_LOG = CLIArgument( + short="-t", + long="--transaction-log", + is_positional=True, + ) + SUPPRESS_TRANSACTION_LOG = CLIArgument( + short="-st", + long="--suppress-transaction-log", + is_positional=True, + ) @classmethod def all(cls) -> List[CLIArgument]: @@ -36,7 +46,13 @@ class MainArguments: ------- List of MainArgument classes """ - return [cls.CONFIG, cls.DRY_RUN, cls.LOG_LEVEL] + return [ + cls.CONFIG, + cls.DRY_RUN, + cls.LOG_LEVEL, + cls.TRANSACTION_LOG, + cls.SUPPRESS_TRANSACTION_LOG, + ] @classmethod def all_arguments(cls) -> List[str]: @@ -91,6 +107,21 @@ def _add_shared_arguments(arg_parser: argparse.ArgumentParser, suppress_defaults choices=LoggerLevels.names(), dest="ytdl_sub_log_level", ) + arg_parser.add_argument( + MainArguments.TRANSACTION_LOG.short, + MainArguments.TRANSACTION_LOG.long, + metavar="TRANSACTIONPATH", + type=str, + help="path to store the transaction log output of all files added, modified, deleted", + default=argparse.SUPPRESS if suppress_defaults else "", + ) + arg_parser.add_argument( + MainArguments.SUPPRESS_TRANSACTION_LOG.short, + MainArguments.SUPPRESS_TRANSACTION_LOG.long, + action="store_true", + help="do not output transaction logs to console or file", + default=argparse.SUPPRESS if suppress_defaults else False, + ) ################################################################################################### @@ -98,7 +129,7 @@ def _add_shared_arguments(arg_parser: argparse.ArgumentParser, suppress_defaults parser = argparse.ArgumentParser( description="ytdl-sub: Automate download and adding metadata with YoutubeDL", ) -parser.add_argument("--version", action="version", version="%(prog)s " + __local_version__) +parser.add_argument("-v", "--version", action="version", version="%(prog)s " + __local_version__) _add_shared_arguments(parser, suppress_defaults=False) subparsers = parser.add_subparsers(dest="subparser") diff --git a/tests/unit/cli/test_main.py b/tests/unit/cli/test_main.py index e8a98e7..8dc6e2b 100644 --- a/tests/unit/cli/test_main.py +++ b/tests/unit/cli/test_main.py @@ -1,21 +1,67 @@ import os.path import re import shutil +import sys import tempfile import time from pathlib import Path from typing import Callable +from typing import Optional from unittest.mock import patch import mergedeep import pytest +from conftest import assert_logs from ytdl_sub.cli.main import _download_subscriptions_from_yaml_files +from ytdl_sub.cli.main import logger as main_logger +from ytdl_sub.cli.main import main from ytdl_sub.config.config_file import ConfigFile from ytdl_sub.subscriptions.subscription import Subscription +from ytdl_sub.utils.file_handler import FileHandler from ytdl_sub.utils.file_handler import FileHandlerTransactionLog +from ytdl_sub.utils.file_handler import FileMetadata from ytdl_sub.utils.logger import Logger +#################################################################################################### +# SHARED FIXTURES + + +@pytest.fixture +def mock_subscription_download_factory(): + def _mock_subscription_download_factory(mock_success_output: bool) -> Callable: + def _mock_download(self: Subscription, dry_run: bool) -> FileHandlerTransactionLog: + Logger.get().info( + "name=%s success=%s dry_run=%s", self.name, mock_success_output, dry_run + ) + time.sleep(1) + if not mock_success_output: + raise ValueError("error") + return ( + FileHandlerTransactionLog() + .log_created_file("created_file.txt", FileMetadata()) + .log_modified_file("modified_file.txt", FileMetadata()) + .log_removed_file("deleted_file.txt") + ) + + return _mock_download + + return _mock_subscription_download_factory + + +@pytest.fixture +def mock_subscription_download_success(mock_subscription_download_factory: Callable): + with patch.object( + Subscription, + "download", + new=mock_subscription_download_factory(mock_success_output=True), + ): + yield + + +#################################################################################################### +# PERSIST LOGS FIXTURES + TESTS + @pytest.fixture def persist_logs_directory() -> str: @@ -53,81 +99,150 @@ def persist_logs_config_factory( return _persist_logs_config_factory -@pytest.fixture -def mock_subscription_download_factory(): - def _mock_subscription_download_factory(mock_success_output: bool) -> Callable: - def _mock_download(self: Subscription, dry_run: bool) -> FileHandlerTransactionLog: - Logger.get().info( - "name=%s success=%s dry_run=%s", self.name, mock_success_output, dry_run - ) - time.sleep(1) - if not mock_success_output: - raise ValueError("error") - return FileHandlerTransactionLog() +@pytest.mark.parametrize("dry_run", [True, False]) +@pytest.mark.parametrize("mock_success_output", [True, False]) +@pytest.mark.parametrize("keep_successful_logs", [True, False]) +def test_subscription_logs_write_to_file( + persist_logs_directory: str, + persist_logs_config_factory: Callable, + mock_subscription_download_factory: Callable, + music_video_subscription_path: Path, + dry_run: bool, + mock_success_output: bool, + keep_successful_logs: bool, +): + num_subscriptions = 2 + config = persist_logs_config_factory(keep_successful_logs=keep_successful_logs) + subscription_paths = [str(music_video_subscription_path)] * num_subscriptions - return _mock_download - - return _mock_subscription_download_factory - - -class TestPersistLogs: - @pytest.mark.parametrize("dry_run", [True, False]) - @pytest.mark.parametrize("mock_success_output", [True, False]) - @pytest.mark.parametrize("keep_successful_logs", [True, False]) - def test_subscription_logs_write_to_file( - self, - persist_logs_directory: str, - persist_logs_config_factory: Callable, - mock_subscription_download_factory: Callable, - music_video_subscription_path: Path, - dry_run: bool, - mock_success_output: bool, - keep_successful_logs: bool, + with patch.object( + Subscription, + "download", + new=mock_subscription_download_factory(mock_success_output=mock_success_output), ): - num_subscriptions = 2 - config = persist_logs_config_factory(keep_successful_logs=keep_successful_logs) - subscription_paths = [str(music_video_subscription_path)] * num_subscriptions + try: + _download_subscriptions_from_yaml_files( + config=config, subscription_paths=subscription_paths, dry_run=dry_run + ) + except ValueError: + assert not mock_success_output - with patch.object( - Subscription, - "download", - new=mock_subscription_download_factory(mock_success_output=mock_success_output), - ): - try: - _download_subscriptions_from_yaml_files( - config=config, subscription_paths=subscription_paths, dry_run=dry_run - ) - except ValueError: - assert not mock_success_output + log_directory_files = list(Path(persist_logs_directory).rglob("*")) - log_directory_files = list(Path(persist_logs_directory).rglob("*")) + # If dry run or success but success logging disabled, expect 0 log files + if dry_run or (mock_success_output and not keep_successful_logs): + assert len(log_directory_files) == 0 + return + # If not success, expect 1 log file + elif not mock_success_output: + assert len(log_directory_files) == 1 + log_path = log_directory_files[0] + assert bool(re.match(r"\d{4}-\d{2}-\d{2}-\d{6}\.john_smith\.error\.log", log_path.name)) + with open(log_path, "r", encoding="utf-8") as log_file: + assert log_file.readlines()[-1] == ( + f"Please upload the error log file '{str(log_path)}' and make a Github issue " + f"at https://github.com/jmbannon/ytdl-sub/issues with your config and " + f"command/subscription yaml file to reproduce. Thanks for trying ytdl-sub!\n" + ) + # If success and success logging, expect 3 log files + else: + assert len(log_directory_files) == num_subscriptions + for log_file_path in log_directory_files: + assert bool( + re.match(r"\d{4}-\d{2}-\d{2}-\d{6}\.john_smith\.success\.log", log_file_path.name) + ) + with open(log_file_path, "r", encoding="utf-8") as log_file: + assert ( + log_file.readlines()[-1] + == "[ytdl-sub] name=john_smith success=True dry_run=False\n" + ) - # If dry run or success but success logging disabled, expect 0 log files - if dry_run or (mock_success_output and not keep_successful_logs): - assert len(log_directory_files) == 0 - return - # If not success, expect 1 log file - elif not mock_success_output: - assert len(log_directory_files) == 1 - log_path = log_directory_files[0] - assert bool(re.match(r"\d{4}-\d{2}-\d{2}-\d{6}\.john_smith\.error\.log", log_path.name)) - with open(log_path, "r", encoding="utf-8") as log_file: - assert log_file.readlines()[-1] == ( - f"Please upload the error log file '{str(log_path)}' and make a Github issue " - f"at https://github.com/jmbannon/ytdl-sub/issues with your config and " - f"command/subscription yaml file to reproduce. Thanks for trying ytdl-sub!\n" - ) - # If success and success logging, expect 3 log files - else: - assert len(log_directory_files) == num_subscriptions - for log_file_path in log_directory_files: - assert bool( - re.match( - r"\d{4}-\d{2}-\d{2}-\d{6}\.john_smith\.success\.log", log_file_path.name - ) - ) - with open(log_file_path, "r", encoding="utf-8") as log_file: - assert ( - log_file.readlines()[-1] - == "[ytdl-sub] name=john_smith success=True dry_run=False\n" - ) + +#################################################################################################### +# TRANSACTION LOGS FIXTURES + TESTS + + +@pytest.fixture +def transaction_log_file_path() -> str: + # Delete the temp_file on creation + with tempfile.NamedTemporaryFile() as temp_file: + pass + + yield temp_file.name + + if os.path.isfile(temp_file.name): + FileHandler.delete(temp_file.name) + + +@pytest.mark.parametrize("file_transaction_log", [None, "output.log"]) +def test_suppress_transaction_log( + mock_subscription_download_success, + music_video_config_path: Path, + music_video_subscription_path: Path, + file_transaction_log: Optional[str], +) -> None: + with patch.object( + sys, + "argv", + [ + "ytdl-sub", + "--config", + str(music_video_config_path), + "sub", + str(music_video_subscription_path), + "--suppress-transaction-log", + ] + + (["--transaction-log", file_transaction_log] if file_transaction_log else []), + ), patch("ytdl_sub.cli.main._output_transaction_log") as mock_transaction_log: + transaction_logs = main() + + assert transaction_logs + assert mock_transaction_log.call_count == 0 + + +def test_transaction_log_to_file( + mock_subscription_download_success, + music_video_config_path: Path, + music_video_subscription_path: Path, + transaction_log_file_path: Path, +) -> None: + with patch.object( + sys, + "argv", + [ + "ytdl-sub", + "--config", + str(music_video_config_path), + "sub", + str(music_video_subscription_path), + "--transaction-log", + str(transaction_log_file_path), + ], + ): + transaction_logs = main() + assert transaction_logs + + with open(transaction_log_file_path, "r", encoding="utf-8") as transaction_log_file: + assert transaction_log_file.readlines()[0] == "Transaction log for john_smith:\n" + + +def test_transaction_log_to_logger( + mock_subscription_download_success, + music_video_config_path: Path, + music_video_subscription_path: Path, +) -> None: + with patch.object( + sys, + "argv", + [ + "ytdl-sub", + "--config", + str(music_video_config_path), + "sub", + str(music_video_subscription_path), + ], + ), assert_logs( + logger=main_logger, expected_message="Transaction log for john_smith:\n", log_level="info" + ): + transaction_logs = main() + assert transaction_logs