From 4105c2ed40fe6c2da4f46ca92e2b413b720900eb Mon Sep 17 00:00:00 2001 From: Jesse Bannon Date: Wed, 18 Oct 2023 09:58:37 -0700 Subject: [PATCH] [FEATURE] No longer require a config.yaml (#763) The only required field in a config was `working_directory`. This is now optional (defaults to `.ytdl-sub-working-directory`) making it so a config is no longer required. This sets the stage to onboard noob users who will solely use prebuilt presets in a single subscription file --- src/ytdl_sub/cli/main.py | 10 +++++- src/ytdl_sub/cli/main_args_parser.py | 6 ++-- src/ytdl_sub/config/config_file.py | 11 +++--- src/ytdl_sub/config/config_validator.py | 16 +++++---- tests/e2e/plugins/internal/test_view.py | 3 +- tests/e2e/youtube/test_playlist.py | 5 ++- tests/unit/main/test_main.py | 46 +++++++++++++++++++++++++ 7 files changed, 77 insertions(+), 20 deletions(-) diff --git a/src/ytdl_sub/cli/main.py b/src/ytdl_sub/cli/main.py index fbace307..4ed5d6a3 100644 --- a/src/ytdl_sub/cli/main.py +++ b/src/ytdl_sub/cli/main.py @@ -11,6 +11,7 @@ from colorama import Fore from yt_dlp.utils import sanitize_filename from ytdl_sub.cli.download_args_parser import DownloadArgsParser +from ytdl_sub.cli.main_args_parser import DEFAULT_CONFIG_FILE_NAME from ytdl_sub.cli.main_args_parser import parser from ytdl_sub.config.config_file import ConfigFile from ytdl_sub.subscriptions.subscription import Subscription @@ -303,7 +304,14 @@ def main() -> List[Tuple[Subscription, FileHandlerTransactionLog]]: args, extra_args = parser.parse_known_args() # Load the config - config: ConfigFile = ConfigFile.from_file_path(args.config) + config: ConfigFile = ConfigFile(name="default_config", value={}) + if args.config: + config = ConfigFile.from_file_path(args.config) + elif os.path.isfile(DEFAULT_CONFIG_FILE_NAME): + config = ConfigFile.from_file_path(DEFAULT_CONFIG_FILE_NAME) + else: + logger.info("No config specified, using defaults.") + transaction_logs: List[Tuple[Subscription, FileHandlerTransactionLog]] = [] # If transaction log file is specified, make sure we can open it diff --git a/src/ytdl_sub/cli/main_args_parser.py b/src/ytdl_sub/cli/main_args_parser.py index 35f63849..ccb50734 100644 --- a/src/ytdl_sub/cli/main_args_parser.py +++ b/src/ytdl_sub/cli/main_args_parser.py @@ -5,6 +5,8 @@ from typing import List from ytdl_sub import __local_version__ from ytdl_sub.utils.logger import LoggerLevels +DEFAULT_CONFIG_FILE_NAME: str = "config.yaml" + @dataclasses.dataclass class CLIArgument: @@ -86,8 +88,8 @@ def _add_shared_arguments(arg_parser: argparse.ArgumentParser, suppress_defaults MainArguments.CONFIG.long, metavar="CONFIGPATH", type=str, - help="path to the config yaml, uses config.yaml if not provided", - default=argparse.SUPPRESS if suppress_defaults else "config.yaml", + help=f"path to the config yaml, uses {DEFAULT_CONFIG_FILE_NAME} if not provided", + default=argparse.SUPPRESS if suppress_defaults else None, # Default is set downstream ) arg_parser.add_argument( MainArguments.DRY_RUN.short, diff --git a/src/ytdl_sub/config/config_file.py b/src/ytdl_sub/config/config_file.py index b5981e51..da79e93f 100644 --- a/src/ytdl_sub/config/config_file.py +++ b/src/ytdl_sub/config/config_file.py @@ -11,8 +11,6 @@ from ytdl_sub.validators.file_path_validators import FilePathValidatorMixin class ConfigFile(ConfigValidator): - _required_keys = {"configuration", "presets"} - def __init__(self, name: str, value: Any): super().__init__(name, value) @@ -45,18 +43,19 @@ class ConfigFile(ConfigValidator): return self @classmethod - def from_dict(cls, config_dict: dict) -> "ConfigFile": + def from_dict(cls, config_dict: dict, name: str = "") -> "ConfigFile": """ Parameters ---------- config_dict: The config in dictionary format - + name: + Name of the config Returns ------- Config file validator """ - return ConfigFile(name="", value=config_dict) + return ConfigFile(name=name, value=config_dict) @classmethod def from_file_path(cls, config_path: str) -> "ConfigFile": @@ -83,7 +82,7 @@ class ConfigFile(ConfigValidator): f"Did you set --config correctly?" ) from exc - return ConfigFile.from_dict(config_dict) + return ConfigFile.from_dict(name=config_path, config_dict=config_dict) def as_dict(self) -> Dict[str, Any]: """ diff --git a/src/ytdl_sub/config/config_validator.py b/src/ytdl_sub/config/config_validator.py index 20341088..1f47d878 100644 --- a/src/ytdl_sub/config/config_validator.py +++ b/src/ytdl_sub/config/config_validator.py @@ -96,8 +96,8 @@ class PersistLogsValidator(StrictDictValidator): class ConfigOptions(StrictDictValidator): - _required_keys = {"working_directory"} _optional_keys = { + "working_directory", "umask", "dl_aliases", "persist_logs", @@ -112,8 +112,10 @@ class ConfigOptions(StrictDictValidator): def __init__(self, name: str, value: Any): super().__init__(name, value) - self._working_directory = self._validate_key( - key="working_directory", validator=StringValidator + self._working_directory = self._validate_key_if_present( + key="working_directory", + validator=StringValidator, + default=".ytdl-sub-working-directory", ) self._umask = self._validate_key_if_present( key="umask", validator=StringValidator, default="022" @@ -244,14 +246,16 @@ class ConfigOptions(StrictDictValidator): class ConfigValidator(StrictDictValidator): - _required_keys = {"configuration", "presets"} + _optional_keys = {"configuration", "presets"} def __init__(self, name: str, value: Any): super().__init__(name, value) - self.config_options = self._validate_key("configuration", ConfigOptions) + self.config_options = self._validate_key_if_present( + "configuration", ConfigOptions, default={} + ) # Make sure presets is a dictionary. Will be validated in `PresetValidator` - self.presets = self._validate_key("presets", LiteralDictValidator) + self.presets = self._validate_key_if_present("presets", LiteralDictValidator, default={}) # Ensure custom presets do not collide with prebuilt presets for preset_name in self.presets.keys: diff --git a/tests/e2e/plugins/internal/test_view.py b/tests/e2e/plugins/internal/test_view.py index 590bfc6e..e3838e6f 100644 --- a/tests/e2e/plugins/internal/test_view.py +++ b/tests/e2e/plugins/internal/test_view.py @@ -10,12 +10,11 @@ class TestView: @pytest.mark.parametrize("split_chapters", [True, False]) def test_view_from_cli( self, - music_video_config_path, output_directory, split_chapters, ): - args = f"--config {music_video_config_path} view " + args = f"view " args += "--split-chapters " if split_chapters else "" args += f"https://www.youtube.com/playlist?list=PLBsm_SagFMmdWnCnrNtLjA9kzfrRkto4i" subscription_transaction_log = mock_run_from_cli(args=args) diff --git a/tests/e2e/youtube/test_playlist.py b/tests/e2e/youtube/test_playlist.py index f8899777..4ec810f9 100644 --- a/tests/e2e/youtube/test_playlist.py +++ b/tests/e2e/youtube/test_playlist.py @@ -150,19 +150,18 @@ class TestPlaylist: ) @pytest.mark.parametrize("dry_run", [True, False]) - def test_playlist_download_from_cli_sub( + def test_playlist_download_from_cli_sub_no_provided_config( self, preset_dict_to_subscription_yaml_generator, - music_video_config_for_cli, playlist_preset_dict, output_directory, dry_run, ): + # No config needed when using only prebuilt presets with preset_dict_to_subscription_yaml_generator( subscription_name="music_video_playlist_test", preset_dict=playlist_preset_dict ) as subscription_path: args = "--dry-run " if dry_run else "" - args += f"--config {music_video_config_for_cli} " args += f"sub {subscription_path}" subscription_transaction_log = mock_run_from_cli(args=args) diff --git a/tests/unit/main/test_main.py b/tests/unit/main/test_main.py index b8bb53e2..230ea3b2 100644 --- a/tests/unit/main/test_main.py +++ b/tests/unit/main/test_main.py @@ -1,5 +1,6 @@ import contextlib import logging +import os.path import sys from unittest.mock import patch @@ -7,7 +8,10 @@ import pytest from src.ytdl_sub import __local_version__ from src.ytdl_sub.main import main +from ytdl_sub.cli.main_args_parser import DEFAULT_CONFIG_FILE_NAME +from ytdl_sub.config.config_file import ConfigFile from ytdl_sub.utils.exceptions import ValidationException +from ytdl_sub.utils.file_handler import FileHandler from ytdl_sub.utils.logger import Logger from ytdl_sub.utils.logger import LoggerLevels @@ -94,9 +98,51 @@ def test_args_after_sub_work(mock_sys_exit): assert mock_sub.call_count == 1 assert mock_sub.call_args.kwargs["subscription_paths"] == ["subscriptions.yaml"] + assert mock_sub.call_args.kwargs["config"]._name == "examples/tv_show_config.yaml" assert Logger._LOGGER_LEVEL == LoggerLevels.VERBOSE +def test_no_config_works(mock_sys_exit): + with mock_sys_exit(expected_exit_code=0), patch.object( + sys, + "argv", + ["ytdl-sub", "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 mock_sub.call_args.kwargs["config"]._name == "default_config" + assert Logger._LOGGER_LEVEL == LoggerLevels.VERBOSE + + +def test_uses_default_config_if_present(mock_sys_exit): + # If a config exists in the ytdl-sub root dir, just use that and do not delete it + preexisting_default_config = os.path.isfile(DEFAULT_CONFIG_FILE_NAME) + if not preexisting_default_config: + open(DEFAULT_CONFIG_FILE_NAME, "a").close() + + try: + with mock_sys_exit(expected_exit_code=0), patch.object( + sys, + "argv", + ["ytdl-sub", "sub", "--log-level", "verbose"], + ), patch( + "ytdl_sub.cli.main._download_subscriptions_from_yaml_files" + ) as mock_sub, patch.object( + ConfigFile, "from_file_path", new=lambda _: ConfigFile(name="test default", value={}) + ): + main() + + assert mock_sub.call_count == 1 + assert mock_sub.call_args.kwargs["subscription_paths"] == ["subscriptions.yaml"] + assert mock_sub.call_args.kwargs["config"]._name == "test default" + assert Logger._LOGGER_LEVEL == LoggerLevels.VERBOSE + finally: + if not preexisting_default_config: + FileHandler.delete(DEFAULT_CONFIG_FILE_NAME) + + def test_no_positional_arg_command(mock_sys_exit): with mock_sys_exit(expected_exit_code=1), patch.object( sys,