diff --git a/docs/config.rst b/docs/config.rst index 4e3325a0..b5902478 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -29,7 +29,22 @@ and subscriptions. .. autoclass:: ytdl_sub.config.config_validator.ConfigOptions() :members: :member-order: bysource + :exclude-members: persist_logs +persist_logs +"""""""""""" +Within ``configuration``, define whether logs from subscription downloads +should be persisted. + +.. code-block:: yaml + + configuration: + persist_logs: + logs_directory: "/path/to/log/directory" + +.. autoclass:: ytdl_sub.config.config_validator.PersistLogsValidator() + :members: + :member-order: bysource presets ^^^^^^^ diff --git a/src/ytdl_sub/cli/main.py b/src/ytdl_sub/cli/main.py index a0dfaa50..2e9d1f48 100644 --- a/src/ytdl_sub/cli/main.py +++ b/src/ytdl_sub/cli/main.py @@ -1,4 +1,3 @@ -import argparse import gc import sys from datetime import datetime @@ -26,15 +25,24 @@ _VIEW_EXTRA_ARGS_FORMATTER = "--preset _view --overrides.url {}" def _maybe_write_subscription_log_file( - config: ConfigFile, subscription: Subscription, success: bool -) -> Optional[Path]: + config: ConfigFile, + subscription: Subscription, + dry_run: bool, + exception: Optional[Exception] = None, +) -> None: + success: bool = exception is None + + # If dry-run, do nothing + if dry_run: + return + # If persisting logs is disabled, do nothing if not config.config_options.persist_logs: - return None + return # If persisting successful logs is disabled, do nothing if success and not config.config_options.persist_logs.keep_successful_logs: - return None + return log_time = datetime.now().strftime("%Y-%m-%d-%H%M%S") log_subscription_name = sanitize_filename(subscription.name).lower().replace(" ", "_") @@ -43,12 +51,14 @@ def _maybe_write_subscription_log_file( 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) + FileHandler.copy(Logger.debug_log_filename(), persist_log_path) - return persist_log_path def _download_subscriptions_from_yaml_files( - config: ConfigFile, args: argparse.Namespace + config: ConfigFile, subscription_paths: List[str], dry_run: bool ) -> List[Tuple[Subscription, FileHandlerTransactionLog]]: """ Downloads all subscriptions from one or many subscription yaml files. @@ -57,8 +67,10 @@ def _download_subscriptions_from_yaml_files( ---------- config Configuration file - args - Arguments from argparse + subscription_paths + Path to subscription files to download + dry_run + Whether to dry run or not Returns ------- @@ -69,7 +81,6 @@ def _download_subscriptions_from_yaml_files( Exception Any exception during download """ - subscription_paths: List[str] = args.subscription_paths subscriptions: List[Subscription] = [] output: List[Tuple[Subscription, FileHandlerTransactionLog]] = [] @@ -80,23 +91,22 @@ def _download_subscriptions_from_yaml_files( for subscription in subscriptions: logger.info( "Beginning subscription %s for %s", - ("dry run" if args.dry_run else "download"), + ("dry run" if dry_run else "download"), subscription.name, ) logger.debug("Subscription full yaml:\n%s", subscription.as_yaml()) try: - transaction_log = subscription.download(dry_run=args.dry_run) + transaction_log = subscription.download(dry_run=dry_run) except Exception as exc: # pylint: disable=broad-except - persisted_log_path = _maybe_write_subscription_log_file( - config=config, subscription=subscription, success=False + _maybe_write_subscription_log_file( + config=config, subscription=subscription, dry_run=dry_run, exception=exc ) - Logger.log_exit_exception(exception=exc, log_filepath=persisted_log_path) raise else: output.append((subscription, transaction_log)) _maybe_write_subscription_log_file( - config=config, subscription=subscription, success=True + config=config, subscription=subscription, dry_run=dry_run ) Logger.cleanup() # Cleanup logger after each successful subscription download finally: @@ -177,7 +187,11 @@ def main() -> List[Tuple[Subscription, FileHandlerTransactionLog]]: with working_directory_lock(config=config): if args.subparser == "sub": - transaction_logs = _download_subscriptions_from_yaml_files(config=config, args=args) + transaction_logs = _download_subscriptions_from_yaml_files( + config=config, + subscription_paths=args.subscription_paths, + dry_run=args.dry_run, + ) # One-off download elif args.subparser == "dl": diff --git a/src/ytdl_sub/config/config_file.py b/src/ytdl_sub/config/config_file.py index 00a03efe..d7874e28 100644 --- a/src/ytdl_sub/config/config_file.py +++ b/src/ytdl_sub/config/config_file.py @@ -1,5 +1,6 @@ import os from typing import Any +from typing import Dict from ytdl_sub.config.config_validator import ConfigValidator from ytdl_sub.config.preset import Preset @@ -65,3 +66,11 @@ class ConfigFile(ConfigValidator): """ config_dict = load_yaml(file_path=config_path) return ConfigFile.from_dict(config_dict) + + def as_dict(self) -> Dict[str, Any]: + """ + Returns + ------- + The config in its dict form + """ + return self._value diff --git a/src/ytdl_sub/config/config_validator.py b/src/ytdl_sub/config/config_validator.py index 4b029efb..48b76409 100644 --- a/src/ytdl_sub/config/config_validator.py +++ b/src/ytdl_sub/config/config_validator.py @@ -48,14 +48,31 @@ class PersistLogsValidator(StrictDictValidator): @property def logs_directory(self) -> str: + """ + Required. The directory to store the logs in. + """ return self._logs_directory.value @property def keep_logs_after(self) -> Optional[str]: + """ + Optional. Keep logs after this date, in yt-dlp datetime format. + + .. code-block:: Markdown + + A string in the format YYYYMMDD or + (now|today|yesterday|date)[+-][0-9](microsecond|second|minute|hour|day|week|month|year)(s) + + For example, ``today-1week`` means keep 1 week's worth of logs. By default, ytdl-sub will + keep all log files. + """ return self._keep_logs_after @property def keep_successful_logs(self) -> bool: + """ + Optional. Whether to store logs when downloading is successful. Defaults to True. + """ return self._keep_successful_logs.value @@ -142,7 +159,9 @@ class ConfigOptions(StrictDictValidator): @property def persist_logs(self) -> Optional[PersistLogsValidator]: - # TODO: nested docstring??? + """ + Persist logs validator. readthedocs in the validator itself! + """ return self._persist_logs @property diff --git a/tests/conftest.py b/tests/conftest.py index 3e2be58e..0571d612 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -139,7 +139,7 @@ def preset_dict_to_subscription_yaml_generator() -> Callable: # Example config fixtures -def _load_config(config_path: Path, working_directory: Path) -> ConfigFile: +def _load_config(config_path: Path, working_directory: str) -> ConfigFile: config_dict = load_yaml(file_path=config_path) config_dict["configuration"]["working_directory"] = working_directory @@ -156,6 +156,11 @@ def music_video_config(music_video_config_path, working_directory) -> ConfigFile return _load_config(music_video_config_path, working_directory) +@pytest.fixture() +def music_video_subscription_path() -> Path: + return Path("examples/music_videos_subscriptions.yaml") + + @pytest.fixture() def channel_as_tv_show_config(working_directory) -> ConfigFile: return _load_config( diff --git a/tests/unit/cli/test_main.py b/tests/unit/cli/test_main.py index 67f305f6..50d74f81 100644 --- a/tests/unit/cli/test_main.py +++ b/tests/unit/cli/test_main.py @@ -1,24 +1,18 @@ -import sys +import re import tempfile +import time +from pathlib import Path +from typing import Callable from unittest.mock import patch +import mergedeep import pytest -from ytdl_sub.cli.main import main - - -def test_args_after_sub_work(): - with patch.object( - sys, - "argv", - ["ytdl-sub", "-c", "examples/tv_show_config.yaml", "sub", "--log-level", "debug"], - ), patch("ytdl_sub.cli.main._download_subscriptions_from_yaml_files") as mock_sub: - main() - - assert mock_sub.call_count == 1 - assert mock_sub.call_args.kwargs["args"].config == "examples/tv_show_config.yaml" - assert mock_sub.call_args.kwargs["args"].subscription_paths == ["subscriptions.yaml"] - assert mock_sub.call_args.kwargs["args"].ytdl_sub_log_level == "debug" +from ytdl_sub.cli.main import _download_subscriptions_from_yaml_files +from ytdl_sub.config.config_file import ConfigFile +from ytdl_sub.subscriptions.subscription import Subscription +from ytdl_sub.utils.file_handler import FileHandlerTransactionLog +from ytdl_sub.utils.logger import Logger @pytest.fixture @@ -28,21 +22,104 @@ def persist_logs_directory(): @pytest.fixture -def persist_logs_config(): - pass +def persist_logs_config_factory( + music_video_config: ConfigFile, persist_logs_directory: str +) -> Callable: + def _persist_logs_config_factory(keep_successful_logs: bool) -> ConfigFile: + return ConfigFile.from_dict( + dict( + mergedeep.merge( + music_video_config.as_dict(), + { + "configuration": { + "persist_logs": { + "logs_directory": persist_logs_directory, + "keep_successful_logs": keep_successful_logs, + }, + } + }, + ) + ) + ) + + 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() + + 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, mock_success_output: bool, keep_successful_logs: bool + 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, ): + num_subscriptions = 2 + config = persist_logs_config_factory(keep_successful_logs=keep_successful_logs) + subscription_paths = [str(music_video_subscription_path)] * num_subscriptions - config_dict = { - "working_directory": ".", - "persist_logs": { - "logs_directory": persist_logs_directory, - "keep_successful_logs": keep_successful_logs, - }, - } + 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("*")) + + # 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" + ) diff --git a/tests/unit/main/test_main.py b/tests/unit/main/test_main.py index 6e6a3e1a..368d2def 100644 --- a/tests/unit/main/test_main.py +++ b/tests/unit/main/test_main.py @@ -9,6 +9,7 @@ from src.ytdl_sub import __local_version__ from src.ytdl_sub.main import main from ytdl_sub.utils.exceptions import ValidationException from ytdl_sub.utils.logger import Logger +from ytdl_sub.utils.logger import LoggerLevels @pytest.fixture @@ -66,3 +67,16 @@ def test_main_uncaught_error(capsys, mock_sys_exit, expected_uncaught_error_mess assert mock_error.call_args.args[0] == expected_uncaught_error_message assert mock_error.call_args.args[1] == __local_version__ assert mock_error.call_args.args[2] == Logger.debug_log_filename() + + +def test_args_after_sub_work(mock_sys_exit): + with mock_sys_exit(expected_exit_code=0), patch.object( + sys, + "argv", + ["ytdl-sub", "-c", "examples/tv_show_config.yaml", "sub", "--log-level", "verbose"], + ), patch("ytdl_sub.cli.main._download_subscriptions_from_yaml_files") as mock_sub: + main() + + assert mock_sub.call_count == 1 + assert mock_sub.call_args.kwargs["subscription_paths"] == ["subscriptions.yaml"] + assert Logger._LOGGER_LEVEL == LoggerLevels.VERBOSE