[BUGFIX] Read parent preset download strategy when partial validating (#566)
This commit is contained in:
parent
4b9709aeab
commit
c8de12833e
2 changed files with 97 additions and 19 deletions
|
|
@ -154,7 +154,9 @@ class _PresetShell(StrictDictValidator):
|
|||
|
||||
class Preset(_PresetShell):
|
||||
@classmethod
|
||||
def _validate_download_strategy(cls, name: str, value: Dict) -> None:
|
||||
def _validate_download_strategy(
|
||||
cls, config: ConfigValidator, name: str, value: Dict, parent_presets: StringListValidator
|
||||
) -> None:
|
||||
sources: List[str] = []
|
||||
for source_name in DownloadStrategyMapping.sources():
|
||||
if source_name in value:
|
||||
|
|
@ -170,9 +172,42 @@ class Preset(_PresetShell):
|
|||
if not sources:
|
||||
return
|
||||
|
||||
# Ensure all parent/child presets use the same download strategy
|
||||
preset_strs = [preset.value for preset in parent_presets.list] + [name]
|
||||
download_strategy: Optional[str] = None
|
||||
download_strategy_preset: Optional[str] = None
|
||||
for parent_preset in preset_strs:
|
||||
# See if the parent has a download strategy
|
||||
parent_download_strategy = (
|
||||
config.presets.dict.get(parent_preset, {})
|
||||
.get("download", {})
|
||||
.get("download_strategy")
|
||||
)
|
||||
|
||||
# If the parent download strategy is None, do nothing
|
||||
if parent_download_strategy is None:
|
||||
continue
|
||||
|
||||
# If download strategy is not set, then set it to parent download strategy
|
||||
if download_strategy is None:
|
||||
download_strategy = parent_download_strategy
|
||||
download_strategy_preset = parent_preset
|
||||
|
||||
# If it's set and does not match the parent download strategy, error
|
||||
if download_strategy is not None and download_strategy != parent_download_strategy:
|
||||
raise ValidationException(
|
||||
f"Preset {download_strategy_preset} uses download strategy {download_strategy}"
|
||||
f", but is inherited by preset {parent_preset} which uses download strategy "
|
||||
f"{parent_download_strategy}."
|
||||
)
|
||||
|
||||
source_name = sources[0]
|
||||
source_dict = copy.deepcopy(value[source_name])
|
||||
|
||||
# Set the parent download strategy if present
|
||||
if download_strategy:
|
||||
source_dict["download_strategy"] = download_strategy
|
||||
|
||||
downloader = DownloadStrategyValidator(name=f"{name}.{source_name}", value=source_dict).get(
|
||||
downloader_source=source_name
|
||||
)
|
||||
|
|
@ -182,14 +217,6 @@ class Preset(_PresetShell):
|
|||
name=f"{name}.{source_name}", value=source_dict
|
||||
)
|
||||
|
||||
if source_name != "download":
|
||||
logger.warning(
|
||||
"WARNING: Download strategy '%s' is deprecated and will be removed in ytdl-sub"
|
||||
"v0.6.0. See %s on how to update it",
|
||||
source_name,
|
||||
"https://ytdl-sub.readthedocs.io/en/latest/config.html#deprecation-updates",
|
||||
)
|
||||
|
||||
@classmethod
|
||||
def preset_partial_validate(cls, config: ConfigValidator, name: str, value: Any) -> None:
|
||||
"""
|
||||
|
|
@ -215,7 +242,18 @@ class Preset(_PresetShell):
|
|||
_ = _PresetShell(name=name, value=value)
|
||||
assert isinstance(value, dict)
|
||||
|
||||
cls._validate_download_strategy(name, value)
|
||||
parent_presets = StringListValidator(name=f"{name}.preset", value=value.get("preset", []))
|
||||
for parent_preset_name in parent_presets.list:
|
||||
if parent_preset_name.value not in config.presets.keys:
|
||||
raise _parent_preset_error_message(
|
||||
current_preset_name=name,
|
||||
parent_preset_name=parent_preset_name.value,
|
||||
presets=config.presets.keys,
|
||||
)
|
||||
|
||||
cls._validate_download_strategy(
|
||||
config=config, name=name, value=value, parent_presets=parent_presets
|
||||
)
|
||||
cls._partial_validate_key(name, value, "output_options", OutputOptions)
|
||||
cls._partial_validate_key(name, value, "ytdl_options", YTDLOptions)
|
||||
cls._partial_validate_key(name, value, "overrides", Overrides)
|
||||
|
|
@ -228,15 +266,6 @@ class Preset(_PresetShell):
|
|||
validator=PluginMapping.get(plugin_name).plugin_options_type,
|
||||
)
|
||||
|
||||
parent_presets = StringListValidator(name=f"{name}.preset", value=value.get("preset", []))
|
||||
for parent_preset_name in parent_presets.list:
|
||||
if parent_preset_name.value not in config.presets.keys:
|
||||
raise _parent_preset_error_message(
|
||||
current_preset_name=name,
|
||||
parent_preset_name=parent_preset_name.value,
|
||||
presets=config.presets.keys,
|
||||
)
|
||||
|
||||
@property
|
||||
def _source_variables(self) -> List[str]:
|
||||
return Entry.source_variables()
|
||||
|
|
|
|||
|
|
@ -154,3 +154,52 @@ class TestConfigFilePartiallyValidatesPresets:
|
|||
expected_error_message="Validation error in partial_preset: "
|
||||
"preset 'DNE' does not exist in the provided config.",
|
||||
)
|
||||
|
||||
def test_partial_validate_partial_download_strategy(self):
|
||||
_ = ConfigFile(
|
||||
name="test_partial_validate",
|
||||
value={
|
||||
"configuration": {"working_directory": "."},
|
||||
"presets": {
|
||||
"parent": {"download": {"download_strategy": "url"}},
|
||||
"child": {"preset": "parent", "download": {"url": "should work"}},
|
||||
},
|
||||
},
|
||||
)
|
||||
|
||||
def test_partial_validate_partial_download_strategies(self):
|
||||
_ = ConfigFile(
|
||||
name="test_partial_validate",
|
||||
value={
|
||||
"configuration": {"working_directory": "."},
|
||||
"presets": {
|
||||
"parent": {"download": {"download_strategy": "url"}},
|
||||
"child": {
|
||||
"preset": "parent",
|
||||
"download": {"download_strategy": "url", "url": "should work"},
|
||||
},
|
||||
},
|
||||
},
|
||||
)
|
||||
|
||||
def test_partial_validate_partial_download_strategies_mismatch(self):
|
||||
with pytest.raises(
|
||||
ValidationException,
|
||||
match=re.escape(
|
||||
"Preset parent uses download strategy multi_url, but is inherited by preset child "
|
||||
"which uses download strategy url."
|
||||
),
|
||||
):
|
||||
_ = ConfigFile(
|
||||
name="test_partial_validate",
|
||||
value={
|
||||
"configuration": {"working_directory": "."},
|
||||
"presets": {
|
||||
"parent": {"download": {"download_strategy": "multi_url"}},
|
||||
"child": {
|
||||
"preset": "parent",
|
||||
"download": {"download_strategy": "url", "url": "should work"},
|
||||
},
|
||||
},
|
||||
},
|
||||
)
|
||||
|
|
|
|||
Loading…
Reference in a new issue