From 1375a0fbe334c28f5b73bfc05b0a82c860fff130 Mon Sep 17 00:00:00 2001 From: Jesse Bannon Date: Wed, 27 Dec 2023 10:49:39 -0800 Subject: [PATCH] [BACKEND] Remove subscription `__value__` (#856) Removes the deprecated `__value__` field in the subscriptions file (different from `__preset__` which is NOT deprecated). This was very short-lived, and most likely not used. --- src/ytdl_sub/config/config_validator.py | 13 -- src/ytdl_sub/subscriptions/subscription.py | 37 +---- .../subscriptions/subscription_validators.py | 8 - src/ytdl_sub/subscriptions/utils.py | 2 - tests/unit/config/test_subscription.py | 157 ++---------------- 5 files changed, 13 insertions(+), 204 deletions(-) delete mode 100644 src/ytdl_sub/subscriptions/utils.py diff --git a/src/ytdl_sub/config/config_validator.py b/src/ytdl_sub/config/config_validator.py index 897684ea6..ff18b3fcd 100644 --- a/src/ytdl_sub/config/config_validator.py +++ b/src/ytdl_sub/config/config_validator.py @@ -10,7 +10,6 @@ from ytdl_sub.config.defaults import DEFAULT_LOCK_DIRECTORY from ytdl_sub.config.defaults import MAX_FILE_NAME_BYTES from ytdl_sub.prebuilt_presets import PREBUILT_PRESETS -from ytdl_sub.subscriptions.utils import SUBSCRIPTION_VALUE_CONFIG_KEY 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 @@ -107,7 +106,6 @@ class ConfigOptions(StrictDictValidator): "ffprobe_path", "file_name_max_bytes", "experimental", - SUBSCRIPTION_VALUE_CONFIG_KEY, } def __init__(self, name: str, value: Any): @@ -142,9 +140,6 @@ def __init__(self, name: str, value: Any): self._file_name_max_bytes = self._validate_key( key="file_name_max_bytes", validator=IntValidator, default=MAX_FILE_NAME_BYTES ) - self._subscription_value = self._validate_key_if_present( - key=SUBSCRIPTION_VALUE_CONFIG_KEY, validator=StringValidator - ) @property def working_directory(self) -> str: @@ -237,14 +232,6 @@ def ffprobe_path(self) -> str: """ return self._ffprobe_path.value - @property - def subscription_value(self) -> Optional[str]: - """ - Sets the :ref:`subscription value` for subscription - files that use this config. - """ - return self._subscription_value.value if self._subscription_value else None - class ConfigValidator(StrictDictValidator): _optional_keys = {"configuration", "presets"} diff --git a/src/ytdl_sub/subscriptions/subscription.py b/src/ytdl_sub/subscriptions/subscription.py index 91989411b..50b2f319a 100644 --- a/src/ytdl_sub/subscriptions/subscription.py +++ b/src/ytdl_sub/subscriptions/subscription.py @@ -3,19 +3,16 @@ from typing import Any from typing import Dict from typing import List -from typing import Optional from ytdl_sub.config.config_file import ConfigFile from ytdl_sub.config.preset import Preset from ytdl_sub.subscriptions.subscription_download import SubscriptionDownload from ytdl_sub.subscriptions.subscription_validators import SubscriptionValidator -from ytdl_sub.utils.exceptions import ValidationException from ytdl_sub.utils.logger import Logger from ytdl_sub.utils.yaml import load_yaml from ytdl_sub.validators.validators import LiteralDictValidator FILE_PRESET_APPLY_KEY = "__preset__" -FILE_SUBSCRIPTION_VALUE_KEY = "__value__" logger = Logger.get("subscription") @@ -70,36 +67,12 @@ def from_dict(cls, config: ConfigFile, preset_name: str, preset_dict: Dict) -> " config=config, ) - @classmethod - def _maybe_get_subscription_value( - cls, config: ConfigFile, subscription_dict: Dict - ) -> Optional[str]: - subscription_value_key: Optional[str] = config.config_options.subscription_value - if FILE_SUBSCRIPTION_VALUE_KEY in subscription_dict: - if not isinstance(subscription_dict[FILE_SUBSCRIPTION_VALUE_KEY], str): - raise ValidationException( - f"Using {FILE_SUBSCRIPTION_VALUE_KEY} in a subscription" - f"must be a string that corresponds to an override variable" - ) - - subscription_value_key = subscription_dict[FILE_SUBSCRIPTION_VALUE_KEY] - - if subscription_value_key is not None: - logger.warning( - "Using %s in a subscription will eventually be deprecated in favor of writing " - "to the override variable `subscription_value`. Please update by Dec 2023.", - FILE_SUBSCRIPTION_VALUE_KEY, - ) - return subscription_value_key - @classmethod def from_file_path( cls, config: ConfigFile, subscription_path: str | Path ) -> List["Subscription"]: """ - Loads subscriptions from a file and applies ``__preset__`` to all of them if present. - If a subscription is in the form of key: value, it will set value to the override - variable defined in ``__value__``. + Loads subscriptions from a file. Parameters ---------- @@ -121,9 +94,6 @@ def from_file_path( subscription_dict = load_yaml(file_path=subscription_path) has_file_preset = FILE_PRESET_APPLY_KEY in subscription_dict - file_subscription_value: Optional[str] = cls._maybe_get_subscription_value( - config=config, subscription_dict=subscription_dict - ) # If a file preset is present... if has_file_preset: @@ -138,9 +108,7 @@ def from_file_path( config.presets.dict[FILE_PRESET_APPLY_KEY] = file_preset.dict subscriptions_dict: Dict[str, Any] = { - key: obj - for key, obj in subscription_dict.items() - if key not in [FILE_PRESET_APPLY_KEY, FILE_SUBSCRIPTION_VALUE_KEY] + key: obj for key, obj in subscription_dict.items() if key not in [FILE_PRESET_APPLY_KEY] } subscriptions_dicts = SubscriptionValidator( @@ -149,7 +117,6 @@ def from_file_path( config=config, presets=[], indent_overrides=[], - subscription_value=file_subscription_value, ).subscription_dicts( global_presets_to_apply=[FILE_PRESET_APPLY_KEY] if has_file_preset else [] ) diff --git a/src/ytdl_sub/subscriptions/subscription_validators.py b/src/ytdl_sub/subscriptions/subscription_validators.py index dbcd55160..03f9114bc 100644 --- a/src/ytdl_sub/subscriptions/subscription_validators.py +++ b/src/ytdl_sub/subscriptions/subscription_validators.py @@ -120,7 +120,6 @@ def __init__( config: ConfigFile, presets: List[str], indent_overrides: List[str], - subscription_value: Optional[str], ): super().__init__( name=name, @@ -129,10 +128,6 @@ def __init__( presets=presets, indent_overrides=indent_overrides, ) - - # TODO: Eventually delete in favor of {subscription_value} - if subscription_value: - self._overrides_to_add[subscription_value] = self.value self._overrides_to_add[SUBSCRIPTION_VALUE] = self.value @@ -234,7 +229,6 @@ def __init__( config: ConfigFile, presets: List[str], indent_overrides: List[str], - subscription_value: Optional[str], ): super().__init__(name=name, value=value, presets=presets, indent_overrides=indent_overrides) self._children: List[SubscriptionOutput] = [] @@ -252,7 +246,6 @@ def __init__( config=config, presets=presets, indent_overrides=indent_overrides, - subscription_value=subscription_value, ) ) # Subscription defined as @@ -294,7 +287,6 @@ def __init__( config=config, presets=presets + preset_indent_key.presets, indent_overrides=indent_overrides + preset_indent_key.indent_overrides, - subscription_value=subscription_value, ) ) else: diff --git a/src/ytdl_sub/subscriptions/utils.py b/src/ytdl_sub/subscriptions/utils.py deleted file mode 100644 index b82620a01..000000000 --- a/src/ytdl_sub/subscriptions/utils.py +++ /dev/null @@ -1,2 +0,0 @@ -# Key used in configs, should delete at some point -SUBSCRIPTION_VALUE_CONFIG_KEY = "subscription_value" diff --git a/tests/unit/config/test_subscription.py b/tests/unit/config/test_subscription.py index ba687b6d0..c4309e43c 100644 --- a/tests/unit/config/test_subscription.py +++ b/tests/unit/config/test_subscription.py @@ -9,19 +9,8 @@ from ytdl_sub.config.config_file import ConfigFile from ytdl_sub.plugins.nfo_tags import NfoTagsOptions -from ytdl_sub.subscriptions.subscription import FILE_SUBSCRIPTION_VALUE_KEY from ytdl_sub.subscriptions.subscription import Subscription from ytdl_sub.utils.exceptions import ValidationException -from ytdl_sub.utils.yaml import load_yaml - - -@pytest.fixture -def config_file_with_subscription_value(config_file: ConfigFile): - config_dict = config_file.as_dict() - mergedeep.merge( - config_dict, {"configuration": {"subscription_value": "test_config_subscription_value"}} - ) - return ConfigFile.from_dict(config_dict) @contextmanager @@ -43,8 +32,6 @@ def preset_with_file_preset(youtube_video: Dict, output_options: Dict): "tags": {"key-3": "file_preset"}, }, "overrides": { - "test_file_subscription_value": "original", - "test_config_subscription_value": "original", "subscription_indent_1": "original_1", "subscription_indent_2": "original_2", "current_override": "__preset__", @@ -70,17 +57,6 @@ def preset_with_subscription_value(preset_with_file_preset: Dict): ) -@pytest.fixture -def preset_with_subscription_file_value(preset_with_subscription_value: Dict): - return dict( - preset_with_subscription_value, - **{ - "__value__": "test_file_subscription_value", - "test_value": "is_overwritten", - }, - ) - - @pytest.fixture def preset_with_subscription_value_nested_presets(preset_with_subscription_value: Dict): return dict( @@ -224,97 +200,14 @@ def test_subscription_file_preset_applies(config_file: ConfigFile, preset_with_f assert overrides.get("current_override") == "test_preset" -def test_subscription_file_value_applies( - config_file: ConfigFile, preset_with_subscription_file_value: Dict -): - with mock_load_yaml(preset_dict=preset_with_subscription_file_value): - subs = Subscription.from_file_path(config=config_file, subscription_path="mocked") - assert len(subs) == 2 - - # Test __value__ worked correctly - value_sub = subs[1] - overrides = value_sub.overrides.dict_with_format_strings - assert value_sub.name == "test_value" - - assert overrides.get("test_file_subscription_value") == "is_overwritten" - assert overrides.get("test_file_subscription_value") - assert overrides.get("subscription_value") == "is_overwritten" - assert overrides.get("current_override") == "__preset__" # ensure __preset__ takes precedence - - -def test_subscription_file_value_applies_sub_file_takes_precedence( - config_file_with_subscription_value: ConfigFile, - preset_with_subscription_file_value: Dict, -): - with mock_load_yaml(preset_dict=preset_with_subscription_file_value): - subs = Subscription.from_file_path( - config=config_file_with_subscription_value, subscription_path="mocked" - ) - assert len(subs) == 2 - - # Test __value__ worked correctly - value_sub = subs[1].overrides.dict_with_format_strings - assert value_sub.get("test_file_subscription_value") == "is_overwritten" - assert value_sub.get("test_config_subscription_value") == "original" - assert value_sub.get("subscription_name") == "test_value" - assert value_sub.get("subscription_value") == "is_overwritten" - assert value_sub.get("current_override") == "__preset__" # ensure __preset__ takes precedence - - -def test_subscription_file_value_applies_from_config( - config_file_with_subscription_value: ConfigFile, preset_with_subscription_value: Dict -): - with mock_load_yaml(preset_dict=preset_with_subscription_value): - subs = Subscription.from_file_path( - config=config_file_with_subscription_value, subscription_path="mocked" - ) - assert len(subs) == 2 - - # Test __value__ worked correctly from the config - value_sub = subs[1].overrides.dict_with_format_strings - assert value_sub.get("test_file_subscription_value") == "original" - assert value_sub.get("test_config_subscription_value") == "is_overwritten" - assert value_sub.get("subscription_name") == "test_value" - assert value_sub.get("subscription_value") == "is_overwritten" - assert value_sub.get("current_override") == "__preset__" # ensure __preset__ takes precedence - - -def test_subscription_file_value_applies_from_config_and_nested( - config_file_with_subscription_value: ConfigFile, - preset_with_subscription_value_nested_presets: Dict, -): - with mock_load_yaml(preset_dict=preset_with_subscription_value_nested_presets): - subs = Subscription.from_file_path( - config=config_file_with_subscription_value, subscription_path="mocked" - ) - assert len(subs) == 4 - - # Test __value__ worked correctly from the config - sub_1 = [sub for sub in subs if sub.name == "test_1"][0].overrides.dict_with_format_strings - sub_2_1 = [sub for sub in subs if sub.name == "test_2_1"][0].overrides.dict_with_format_strings - - assert sub_1.get("test_config_subscription_value") == "is_1_overwritten" - assert sub_1.get("subscription_name") == "test_1" - assert sub_1.get("subscription_value") == "is_1_overwritten" - assert sub_1.get("current_override") == "__preset__" # ensure __preset__ takes precedence - - assert sub_2_1.get("test_config_subscription_value") == "is_2_1_overwritten" - assert sub_2_1.get("subscription_name") == "test_2_1" - assert sub_2_1.get("subscription_value") == "is_2_1_overwritten" - assert sub_2_1.get("current_override") == "__preset__" # ensure __preset__ takes precedence - - def test_subscription_list( - config_file_with_subscription_value: ConfigFile, + config_file: ConfigFile, preset_with_subscription_list: Dict, ): with mock_load_yaml(preset_dict=preset_with_subscription_list): - subs = Subscription.from_file_path( - config=config_file_with_subscription_value, subscription_path="mocked" - ) + subs = Subscription.from_file_path(config=config_file, subscription_path="mocked") assert len(subs) == 3 - # Test __value__ worked correctly from the config sub_2_1 = [sub for sub in subs if sub.name == "test_2_1"][0].overrides.dict_with_format_strings assert sub_2_1.get("subscription_name") == "test_2_1" @@ -325,16 +218,13 @@ def test_subscription_list( def test_subscription_overrides_tilda( - config_file_with_subscription_value: ConfigFile, + config_file: ConfigFile, preset_with_subscription_overrides_tilda: Dict, ): with mock_load_yaml(preset_dict=preset_with_subscription_overrides_tilda): - subs = Subscription.from_file_path( - config=config_file_with_subscription_value, subscription_path="mocked" - ) + subs = Subscription.from_file_path(config=config_file, subscription_path="mocked") assert len(subs) == 3 - # Test __value__ worked correctly from the config sub_2_1 = [sub for sub in subs if sub.name == "test_2_1"][0].overrides.dict_with_format_strings assert sub_2_1.get("subscription_name") == "test_2_1" @@ -342,18 +232,15 @@ def test_subscription_overrides_tilda( def test_subscription_file_value_applies_from_config_and_nested_and_indent_variables( - config_file_with_subscription_value: ConfigFile, + config_file: ConfigFile, preset_with_subscription_value_nested_presets_and_indent_variables: Dict, ): with mock_load_yaml( preset_dict=preset_with_subscription_value_nested_presets_and_indent_variables ): - subs = Subscription.from_file_path( - config=config_file_with_subscription_value, subscription_path="mocked" - ) + subs = Subscription.from_file_path(config=config_file, subscription_path="mocked") assert len(subs) == 4 - # Test __value__ worked correctly from the config sub_test_value = [sub for sub in subs if sub.name == "test_value"][ 0 ].overrides.dict_with_format_strings @@ -363,14 +250,12 @@ def test_subscription_file_value_applies_from_config_and_nested_and_indent_varia assert sub_test_value.get("subscription_indent_1") == "original_1" assert sub_test_value.get("subscription_indent_2") == "original_2" - assert sub_1.get("test_config_subscription_value") == "is_1_overwritten" assert sub_1.get("subscription_name") == "test_1" assert sub_1.get("subscription_value") == "is_1_overwritten" assert sub_1.get("subscription_indent_1") == "INDENT_1" assert sub_1.get("subscription_indent_2") == "INDENT_2" assert sub_1.get("current_override") == "__preset__" # ensure __preset__ takes precedence - assert sub_2_1.get("test_config_subscription_value") == "is_2_1_overwritten" assert sub_2_1.get("subscription_name") == "test_2_1" assert sub_2_1.get("subscription_value") == "is_2_1_overwritten" assert sub_2_1.get("subscription_indent_1") == "INDENT_1" @@ -380,7 +265,7 @@ def test_subscription_file_value_applies_from_config_and_nested_and_indent_varia @pytest.mark.parametrize("all_same_line", [True, False]) def test_subscription_file_value_applies_from_config_and_nested_and_indent_variables_same_line( - config_file_with_subscription_value: ConfigFile, + config_file: ConfigFile, preset_with_subscription_value_nested_presets_and_indent_variables_same_line: Dict, preset_with_subscription_value_nested_presets_and_indent_variables_all_same_line: Dict, all_same_line: bool, @@ -392,12 +277,9 @@ def test_subscription_file_value_applies_from_config_and_nested_and_indent_varia ) with mock_load_yaml(preset_dict=preset_dict): - subs = Subscription.from_file_path( - config=config_file_with_subscription_value, subscription_path="mocked" - ) + subs = Subscription.from_file_path(config=config_file, subscription_path="mocked") assert len(subs) == 4 - # Test __value__ worked correctly from the config sub_test_value = [sub for sub in subs if sub.name == "test_value"][ 0 ].overrides.dict_with_format_strings @@ -407,7 +289,6 @@ def test_subscription_file_value_applies_from_config_and_nested_and_indent_varia assert sub_test_value.get("subscription_indent_1") == "original_1" assert sub_test_value.get("subscription_indent_2") == "original_2" - assert sub_1.get("test_config_subscription_value") == "is_1_overwritten" assert sub_1.get("subscription_name") == "test_1" assert sub_1.get("subscription_value") == "is_1_overwritten" assert sub_1.get("subscription_indent_1") == "INDENT_1" @@ -415,7 +296,6 @@ def test_subscription_file_value_applies_from_config_and_nested_and_indent_varia assert sub_1.get("subscription_indent_3") == "INDENT_3" assert sub_1.get("current_override") == "__preset__" # ensure __preset__ takes precedence - assert sub_2_1.get("test_config_subscription_value") == "is_2_1_overwritten" assert sub_2_1.get("subscription_name") == "test_2_1" assert sub_2_1.get("subscription_value") == "is_2_1_overwritten" assert sub_2_1.get("subscription_indent_1") == "INDENT_1" @@ -425,7 +305,7 @@ def test_subscription_file_value_applies_from_config_and_nested_and_indent_varia def test_subscription_file_value_applies_from_config_and_nested_and_indent_variables_same_line_old_format_errors( - config_file_with_subscription_value: ConfigFile, + config_file: ConfigFile, preset_with_subscription_value_nested_presets_and_indent_variables_same_line_old_format_errors: Dict, ): with mock_load_yaml( @@ -437,28 +317,13 @@ def test_subscription_file_value_applies_from_config_and_nested_and_indent_varia "To use as a subscription indent value, define it as '= INDENT_3'" ), ): - Subscription.from_file_path( - config=config_file_with_subscription_value, subscription_path="mocked" - ) - - -def test_subscription_file_bad_value(config_file: ConfigFile): - with mock_load_yaml(preset_dict={"__value__": {"should be": "string"}}), pytest.raises( - ValidationException, - match=re.escape( - f"Using {FILE_SUBSCRIPTION_VALUE_KEY} in a subscription" - f"must be a string that corresponds to an override variable" - ), - ): - _ = Subscription.from_file_path(config=config_file, subscription_path="mocked") + Subscription.from_file_path(config=config_file, subscription_path="mocked") def test_subscription_file_using_conflicting_preset_name(config_file: ConfigFile): with mock_load_yaml( preset_dict={ - "= INDENTS_IN_ERR_MSG ": { - "=ANOTHER": {"jellyfin_tv_show_by_date": "single value, __value__ not defined"} - } + "= INDENTS_IN_ERR_MSG ": {"=ANOTHER": {"jellyfin_tv_show_by_date": "single value"}} } ), pytest.raises( ValidationException,