fix(dl): apply per-service dl config overrides for all options

services.<TAG>.dl values only applied when the key was also set in the global dl: section (equality check against config.dl missed Click's declared defaults). Gate on Click's ParameterSource instead: CLI/env > service dl > global dl > defaults, converted via each option's own type.

- record parameter sources on serve's hand-built context so client values are never clobbered
- accept range/list as natural keys for range_/list_
- harden QualityList (YAML int) and SlowDelayRange (YAML bool) converts
This commit is contained in:
imSp4rky
2026-06-09 18:17:29 -06:00
parent 2f35a4d468
commit 246ff528f5
8 changed files with 302 additions and 47 deletions

View File

@@ -32,8 +32,9 @@ There is no `downloader:` config key to set anymore. Setting one to a legacy val
Pre-define default options and switches of the `dl` command. Pre-define default options and switches of the `dl` command.
The values will be ignored if explicitly set in the CLI call. The values will be ignored if explicitly set in the CLI call.
The Key must be the same value Python click would resolve it to as an argument. The Key is the option name with dashes as underscores (`--v-lang` -> `v_lang`). `range` and `list`
E.g., `@click.option("-r", "--range", "range_", type=...` actually resolves as `range_` variable. work as-is; their internal Python names (`range_`, `list_`, suffixed to avoid the builtins) are
also accepted.
For example to set the default primary language to download to German, For example to set the default primary language to download to German,
@@ -88,7 +89,7 @@ to a CLI option on the `dl` command. CLI arguments always take priority over con
| `abitrate_range` | str | none | Audio bitrate window in kbps, format `MIN-MAX` | | `abitrate_range` | str | none | Audio bitrate window in kbps, format `MIN-MAX` |
| `real_video_bitrate` | bool | `false` | Probe actual media size to compute true video bitrates, overriding the manifest's declared value (`-rvb`). See [Real bitrate probing](#real-bitrate-probing) | | `real_video_bitrate` | bool | `false` | Probe actual media size to compute true video bitrates, overriding the manifest's declared value (`-rvb`). See [Real bitrate probing](#real-bitrate-probing) |
| `real_audio_bitrate` | bool | `false` | Same as above for audio tracks (`-rab`). Slower than video (more renditions) | | `real_audio_bitrate` | bool | `false` | Same as above for audio tracks (`-rab`). Slower than video (more renditions) |
| `range_` | str or list | `SDR` | Color range(s): `SDR`, `HDR10`, `HDR10+`, `HLG`, `DV`, `HYBRID` | | `range` (or `range_`) | str or list | `SDR` | Color range(s): `SDR`, `HDR10`, `HDR10+`, `HLG`, `DV`, `HYBRID` |
| `channels` | float | any | Audio channels (e.g., `5.1`, `7.1`) | | `channels` | float | any | Audio channels (e.g., `5.1`, `7.1`) |
| `worst` | bool | `false` | Select the lowest bitrate track within the specified quality. Requires `quality` | | `worst` | bool | `false` | Select the lowest bitrate track within the specified quality. Requires `quality` |
| `best_available` | bool | `false` | Continue if requested quality is unavailable | | `best_available` | bool | `false` | Continue if requested quality is unavailable |
@@ -186,17 +187,20 @@ How it works:
Per-track before→after values are logged at debug level (run with `-d`); the Per-track before→after values are logged at debug level (run with `-d`); the
corrected values always appear in the Available Tracks panel. corrected values always appear in the Available Tracks panel.
You can also set per-service `dl` overrides (see [Service Integration & Authentication Configuration](SERVICE_CONFIG.md)): You can also set per-service `dl` overrides under the `services` section (see
[Service Integration & Authentication Configuration](SERVICE_CONFIG.md)). Per-service values beat the
global `dl:` section; explicit CLI arguments beat both:
```yaml ```yaml
dl: dl:
lang: en lang: en
downloads: 4 downloads: 4
workers: 16 workers: 16
services:
EXAMPLE: EXAMPLE:
bitrate: CVBR dl:
EXAMPLE2: v_lang: [en]
worst: true
quality: 1080 quality: 1080
``` ```

View File

@@ -30,7 +30,18 @@ service tag in the `services` section. Supported override keys include: `dl`, `s
`headers`, `proxy_map`, `title_map`, and more. `headers`, `proxy_map`, `title_map`, and more.
Overrides are merged with global config (not replaced) -- only specified keys are overridden, others Overrides are merged with global config (not replaced) -- only specified keys are overridden, others
use global defaults. CLI arguments always take priority over service-specific config. use global defaults.
Any `dl` command option can be overridden per service. Use the option name with dashes as underscores
(`--v-lang` -> `v_lang`). `range` and `list` work as-is; their internal Python names (`range_`,
`list_`, suffixed to avoid the builtins) are also accepted.
Precedence (highest first):
1. Explicit CLI arguments / environment variables (e.g. `--v-lang en`)
2. Per-service config (`services.<TAG>.dl`)
3. Global `dl:` config
4. Built-in option defaults
For example, For example,

View File

@@ -6,7 +6,7 @@ from __future__ import annotations
import pytest import pytest
from unshackle.core.tracks.subtitle import Subtitle from unshackle.core.tracks.subtitle import Subtitle
from unshackle.core.utils.click_types import SubtitleCodecChoice from unshackle.core.utils.click_types import QUALITY_LIST, SLOW_DELAY_RANGE, SubtitleCodecChoice
choice = SubtitleCodecChoice(Subtitle.Codec) choice = SubtitleCodecChoice(Subtitle.Codec)
@@ -31,3 +31,29 @@ def test_codecs_still_map(value, expected):
def test_empty_is_none(): def test_empty_is_none():
assert choice.convert(None) is None assert choice.convert(None) is None
@pytest.mark.parametrize(
"value,expected",
[
(1080, [1080]),
([720, 1080], [1080, 720]),
("1080p", [1080]),
("720,1080", [1080, 720]),
],
)
def test_quality_list_accepts_yaml_native_values(value, expected):
assert QUALITY_LIST.convert(value) == expected
@pytest.mark.parametrize(
"value,expected",
[
(True, (60, 120)),
(False, None),
("20-40", (20, 40)),
((25, 30), (25, 30)),
],
)
def test_slow_delay_range_accepts_bool(value, expected):
assert SLOW_DELAY_RANGE.convert(value, None, None) == expected

View File

@@ -0,0 +1,154 @@
"""Tests for ``apply_service_dl_overrides`` — precedence: CLI/env > service dl > global dl > defaults."""
from __future__ import annotations
import logging
from typing import Any, Optional
import click
import pytest
from click.core import ParameterSource
from unshackle.commands.dl import apply_service_dl_overrides, normalize_dl_config
from unshackle.core.tracks import Video
from unshackle.core.utils.click_types import LANGUAGE_RANGE, MultipleChoice
log = logging.getLogger("test_service_dl_overrides")
def make_ctx(args: Optional[list[str]] = None, default_map: Optional[dict[str, Any]] = None) -> click.Context:
"""Build a parsed context for a mirror of the dl group's option shapes."""
@click.group("dl", invoke_without_command=True)
@click.option("-vl", "--v-lang", type=LANGUAGE_RANGE, default=[])
@click.option(
"-r", "--range", "range_", type=MultipleChoice(Video.Range, case_sensitive=False), default=[Video.Range.SDR]
)
@click.option("--repack", is_flag=True, default=False)
@click.option("--workers", type=int, default=None, envvar="TEST_DL_WORKERS")
def cmd(**__: Any) -> None:
pass
return cmd.make_context("dl", args or [], default_map=default_map)
def test_service_only_key_applies():
"""The reported bug: key set only under services.<TAG>.dl, not under global dl:."""
ctx = make_ctx()
assert ctx.params["v_lang"] == []
apply_service_dl_overrides(ctx, {"v_lang": ["en"]}, log)
assert ctx.params["v_lang"] == ["en"]
def test_cli_value_beats_service_config():
ctx = make_ctx(["-vl", "fr"])
apply_service_dl_overrides(ctx, {"v_lang": ["en"]}, log)
assert ctx.params["v_lang"] == ["fr"]
def test_env_value_beats_service_config(monkeypatch):
monkeypatch.setenv("TEST_DL_WORKERS", "5")
ctx = make_ctx()
apply_service_dl_overrides(ctx, {"workers": 9}, log)
assert ctx.params["workers"] == 5
def test_service_config_beats_global_default_map():
ctx = make_ctx(default_map={"v_lang": "de"})
assert ctx.params["v_lang"] == ["de"]
apply_service_dl_overrides(ctx, {"v_lang": ["en"]}, log)
assert ctx.params["v_lang"] == ["en"]
def test_flag_global_true_service_false():
ctx = make_ctx(default_map={"repack": True})
assert ctx.params["repack"] is True
apply_service_dl_overrides(ctx, {"repack": False}, log)
assert ctx.params["repack"] is False
def test_range_scalar_converts_to_enum_list():
ctx = make_ctx()
apply_service_dl_overrides(ctx, {"range_": "HDR10"}, log)
assert ctx.params["range_"] == [Video.Range.HDR10]
def test_range_alias_applies():
"""`range` in config is an accepted alias for the `range_` parameter."""
ctx = make_ctx()
apply_service_dl_overrides(ctx, {"range": "HDR10"}, log)
assert ctx.params["range_"] == [Video.Range.HDR10]
def test_normalize_dl_config_maps_aliases():
assert normalize_dl_config({"range": "SDR", "list": True, "v_lang": ["en"]}) == {
"range_": "SDR",
"list_": True,
"v_lang": ["en"],
}
def test_unknown_key_warns(caplog):
ctx = make_ctx()
with caplog.at_level(logging.WARNING, logger=log.name):
apply_service_dl_overrides(ctx, {"bogus": 1}, log)
assert any("unknown dl option 'bogus'" in r.message for r in caplog.records)
def test_none_value_is_skipped():
ctx = make_ctx()
apply_service_dl_overrides(ctx, {"v_lang": None}, log)
assert ctx.params["v_lang"] == []
def test_invalid_value_warns_and_keeps_current(caplog):
ctx = make_ctx()
with caplog.at_level(logging.WARNING, logger=log.name):
apply_service_dl_overrides(ctx, {"workers": "abc"}, log)
assert ctx.params["workers"] is None
assert any("Failed to apply service dl override 'workers'" in r.message for r in caplog.records)
def hand_built_ctx(body: dict[str, Any]) -> click.Context:
"""Mirror the serve path: ctx.params set directly, sources recorded manually."""
parsed = make_ctx()
ctx = click.Context(parsed.command)
ctx.params = {"repack": body.get("repack", False)}
for name in ctx.params:
ctx.set_parameter_source(
name, ParameterSource.COMMANDLINE if name in body else ParameterSource.DEFAULT
)
return ctx
def test_hand_built_ctx_default_gets_override():
ctx = hand_built_ctx({})
apply_service_dl_overrides(ctx, {"repack": True}, log)
assert ctx.params["repack"] is True
def test_hand_built_ctx_client_value_wins():
ctx = hand_built_ctx({"repack": False})
apply_service_dl_overrides(ctx, {"repack": True}, log)
assert ctx.params["repack"] is False
def test_hand_built_ctx_missing_param_is_skipped():
"""Known dl option absent from a hand-built context must not raise or be injected."""
ctx = hand_built_ctx({})
apply_service_dl_overrides(ctx, {"v_lang": ["en"]}, log)
assert "v_lang" not in ctx.params
@pytest.mark.parametrize(
"value,expected",
[
(["en", "de"], ["en", "de"]),
("en,de", ["en", "de"]),
("en", ["en"]),
],
)
def test_language_range_yaml_shapes(value, expected):
ctx = make_ctx()
apply_service_dl_overrides(ctx, {"v_lang": value}, log)
assert ctx.params["v_lang"] == expected

View File

@@ -25,6 +25,7 @@ from uuid import UUID
import click import click
import yaml import yaml
from click.core import ParameterSource
from langcodes import Language from langcodes import Language
from pymediainfo import MediaInfo from pymediainfo import MediaInfo
from rich.console import Group from rich.console import Group
@@ -80,6 +81,42 @@ class SkippedSubtitle(TypedDict):
title: str title: str
# Config keys accepted as natural names for params whose Python name dodges a builtin.
DL_OPTION_ALIASES = {"range": "range_", "list": "list_"}
def normalize_dl_config(dl_config: dict[str, Any]) -> dict[str, Any]:
"""Map aliased config keys (e.g. ``range``) to their Click parameter names (``range_``)."""
return {DL_OPTION_ALIASES.get(key, key): value for key, value in dl_config.items()}
def apply_service_dl_overrides(ctx: click.Context, service_dl_config: dict[str, Any], log: logging.Logger) -> None:
"""Apply ``services.<TAG>.dl`` config onto ``ctx.params``. Explicit CLI/env values win;
defaults and global ``dl:`` default_map values are replaced."""
params_by_name = {param.name: param for param in ctx.command.params if param.name}
for name, value in normalize_dl_config(service_dl_config).items():
param = params_by_name.get(name)
if param is None:
log.warning(f"Ignoring unknown dl option '{name}' in service config")
continue
if name not in ctx.params:
log.debug(f"Skipping service dl override '{name}': not present in this context")
continue
if value is None:
continue
source = ctx.get_parameter_source(name)
if source not in (ParameterSource.DEFAULT, ParameterSource.DEFAULT_MAP):
continue
try:
ctx.params[name] = param.type_cast_value(ctx, value)
log.debug(f"Applied service dl override '{name}': {ctx.params[name]}")
except Exception as e:
log.warning(
f"Failed to apply service dl override '{name}'={value!r}: {e}. "
f"Check that the value is valid for this parameter."
)
def download_tracks_in_passes( def download_tracks_in_passes(
tracks: Iterable[AnyTrack], tracks: Iterable[AnyTrack],
max_concurrent: int, max_concurrent: int,
@@ -326,7 +363,9 @@ class dl:
@click.command( @click.command(
short_help="Download, Decrypt, and Mux tracks for titles from a Service.", short_help="Download, Decrypt, and Mux tracks for titles from a Service.",
cls=Services, cls=Services,
context_settings=dict(**context_settings, default_map=config.dl, token_normalize_func=Services.get_tag), context_settings=dict(
**context_settings, default_map=normalize_dl_config(config.dl), token_normalize_func=Services.get_tag
),
) )
@click.option( @click.option(
"-p", "--profile", type=str, default=None, help="Profile to use for Credentials and Cookies (if available)." "-p", "--profile", type=str, default=None, help="Profile to use for Credentials and Cookies (if available)."
@@ -687,37 +726,19 @@ class dl:
self.service = Services.get_tag(ctx.invoked_subcommand) self.service = Services.get_tag(ctx.invoked_subcommand)
self.vault_service = Services.get_vault_tag(self.service) self.vault_service = Services.get_vault_tag(self.service)
service_dl_config = config.services.get(self.service, {}).get("dl", {}) apply_service_dl_overrides(ctx, config.services.get(self.service, {}).get("dl", {}), self.log)
if service_dl_config:
param_types = {param.name: param.type for param in ctx.command.params if param.name}
for param_name, service_value in service_dl_config.items(): # Refresh locals Click bound before the overrides ran.
if param_name not in ctx.params: no_proxy = ctx.params.get("no_proxy", no_proxy)
continue profile = ctx.params.get("profile", profile)
proxy = ctx.params.get("proxy", proxy)
current_value = ctx.params[param_name] repack = ctx.params.get("repack", repack)
global_default = config.dl.get(param_name) tag = ctx.params.get("tag", tag)
param_type = param_types.get(param_name) tmdb_id = ctx.params.get("tmdb_id", tmdb_id)
imdb_id = ctx.params.get("imdb_id", imdb_id)
try: animeapi_id = ctx.params.get("animeapi_id", animeapi_id)
if param_type and global_default is not None: enrich = ctx.params.get("enrich", enrich)
global_default = param_type.convert(global_default, None, ctx) output_dir = ctx.params.get("output_dir", output_dir)
except Exception as e:
self.log.debug(f"Failed to convert global default for '{param_name}': {e}")
if current_value == global_default or (current_value is None and global_default is None):
try:
converted_value = service_value
if param_type and service_value is not None:
converted_value = param_type.convert(service_value, None, ctx)
ctx.params[param_name] = converted_value
self.log.debug(f"Applied service-specific '{param_name}' override: {converted_value}")
except Exception as e:
self.log.warning(
f"Failed to apply service-specific '{param_name}' override: {e}. "
f"Check that the value '{service_value}' is valid for this parameter."
)
self.profile = profile self.profile = profile
self.proxy_requested = bool(proxy) self.proxy_requested = bool(proxy)

View File

@@ -292,6 +292,14 @@ def _perform_download(
"no_cache": params.get("no_cache", False), "no_cache": params.get("no_cache", False),
"reset_cache": params.get("reset_cache", False), "reset_cache": params.get("reset_cache", False),
} }
# Hand-built context: record parameter sources so service dl overrides
# apply to defaults but never clobber client-sent values.
from click.core import ParameterSource
for param_name in ctx.params:
ctx.set_parameter_source(
param_name, ParameterSource.COMMANDLINE if param_name in params else ParameterSource.DEFAULT
)
dl_instance = dl( dl_instance = dl(
ctx=ctx, ctx=ctx,

View File

@@ -254,16 +254,19 @@ class QualityList(click.ParamType):
name = "quality_list" name = "quality_list"
def convert( def convert(
self, value: Union[str, list[str]], param: Optional[click.Parameter] = None, ctx: Optional[click.Context] = None self,
value: Union[str, int, list[Union[str, int]]],
param: Optional[click.Parameter] = None,
ctx: Optional[click.Context] = None,
) -> list[int]: ) -> list[int]:
if not value: if not value:
return [] return []
if not isinstance(value, list): if not isinstance(value, list):
value = value.split(",") value = str(value).split(",")
resolutions = [] resolutions = []
for resolution in value: for resolution in value:
try: try:
resolutions.append(int(resolution.lower().rstrip("p"))) resolutions.append(int(str(resolution).lower().rstrip("p")))
except TypeError: except TypeError:
self.fail( self.fail(
f"Expected string for int() conversion, got {resolution!r} of type {type(resolution).__name__}", f"Expected string for int() conversion, got {resolution!r} of type {type(resolution).__name__}",
@@ -370,9 +373,13 @@ class SlowDelayRange(click.ParamType):
name = "delay_range" name = "delay_range"
def convert(self, value: Any, param: Optional[click.Parameter], ctx: Optional[click.Context]) -> tuple[int, int]: def convert(
self, value: Any, param: Optional[click.Parameter], ctx: Optional[click.Context]
) -> Optional[tuple[int, int]]:
if isinstance(value, tuple): if isinstance(value, tuple):
return value return value
if isinstance(value, bool):
return (60, 120) if value else None
match = re.match(r"^(\d+)-(\d+)$", str(value)) match = re.match(r"^(\d+)-(\d+)$", str(value))
if not match: if not match:

View File

@@ -414,6 +414,13 @@ audio:
# Atmos still trumps codec priority. Valid names: AAC, AC3, EC3, AC4, OPUS, OGG, DTS, ALAC, FLAC. # Atmos still trumps codec priority. Valid names: AAC, AC3, EC3, AC4, OPUS, OGG, DTS, ALAC, FLAC.
# codec_priority: [FLAC, ALAC, AC4, EC3, DTS, AC3, OPUS, AAC, OGG] # codec_priority: [FLAC, ALAC, AC4, EC3, DTS, AC3, OPUS, AAC, OGG]
# Global defaults for the dl command. Any dl option can be set here.
# Use the option name with dashes as underscores (--v-lang -> v_lang).
# range and list work as-is (range_/list_ also accepted).
#
# Any of these can also be overridden per service under services.<TAG>.dl
# (see the services section below). Precedence, highest first:
# CLI arguments > services.<TAG>.dl > this dl section > built-in defaults
dl: dl:
sub_format: srt sub_format: srt
downloads: 4 downloads: 4
@@ -421,6 +428,14 @@ dl:
lang: lang:
- en - en
- fr - fr
v_lang: [en] # --v-lang: video language(s), if different from audio
s_lang: [en, de] # --s-lang: subtitle language(s)
range: SDR # --range: SDR, HDR10, HDR10+, DV
quality: [1080] # --quality: resolution(s), int or list
slow: false # --slow: true = random 60-120s delay between titles, or "MIN-MAX" string
# Nesting a SERVICE TAG here provides defaults for that service's OWN command
# options (e.g. a service-defined --bitrate flag) -- NOT for the dl options
# above. To override dl options per service, use services.<TAG>.dl instead.
EXAMPLE: EXAMPLE:
bitrate: CBR bitrate: CBR
@@ -624,12 +639,20 @@ services:
# Other services or no service specified will still use random selection # Other services or no service specified will still use random selection
# NEW: Configuration overrides (can be combined with profiles and certificates) # NEW: Configuration overrides (can be combined with profiles and certificates)
# Override dl command defaults for this service # Override dl command defaults for this service. ANY dl option works here,
# same key naming as the global dl section (v_lang, range_, list_, ...).
# These beat the global dl section; explicit CLI arguments beat both.
dl: dl:
downloads: 4 # Limit concurrent track downloads (global default: 6) downloads: 4 # Limit concurrent track downloads (global default: 6)
workers: 8 # Reduce workers per track (global default: 16) workers: 8 # Reduce workers per track (global default: 16)
lang: ["en", "es-419"] # Different language priority for this service lang: ["en", "es-419"] # Different language priority for this service
v_lang: [en] # Video language differs from audio on this service
s_lang: [all] # Grab every subtitle language here
range: HDR10 # Prefer HDR10 on this service (CLI -r still wins)
quality: 720 # This service rate-limits 1080p; int or list both work
sub_format: srt # Force SRT subtitle format sub_format: srt # Force SRT subtitle format
slow: true # Pace title downloads (60-120s); or "20-40" for a custom range
repack: false # Even if the global dl section sets repack: true
# Override subtitle processing for this service # Override subtitle processing for this service
subtitle: subtitle:
@@ -671,7 +694,8 @@ services:
# - Only specified keys are overridden, others use global defaults # - Only specified keys are overridden, others use global defaults
# - Reserved keys (profiles, api_key, certificate, etc.) are NOT treated as overrides # - Reserved keys (profiles, api_key, certificate, etc.) are NOT treated as overrides
# - Any dict-type config option can be overridden (dl, subtitle, muxing, headers, etc.) # - Any dict-type config option can be overridden (dl, subtitle, muxing, headers, etc.)
# - CLI arguments always take priority over service-specific config # - Any dl option can be overridden; dashes become underscores (--v-lang -> v_lang)
# - Precedence: CLI arguments > service dl override > global dl section > built-in defaults
# External proxy provider services # External proxy provider services
proxy_providers: proxy_providers: