diff --git a/docs/reference/configuration/post-scripts.md b/docs/reference/configuration/post-scripts.md index 9eb043f7..ca4c1980 100644 --- a/docs/reference/configuration/post-scripts.md +++ b/docs/reference/configuration/post-scripts.md @@ -25,6 +25,7 @@ is the same as `{command: "...", event: success, mode: file}`. | `command` | required | The command to run, with `{variable}` placeholders | | `event` | `success` | `success` when a file was written, `failure` when the download failed | | `mode` | `file` | `file`, `season`, or `run`. See [Modes](#modes) | +| `wait` | `false` | `true` makes unshackle wait for the script to exit before carrying on. See [Waiting for a script](#wait) | unshackle skips an entry whose `event` or `mode` is not one of the listed values, and logs a warning that names the bad value, once per command. unshackle also skips an entry whose @@ -66,15 +67,30 @@ means a path containing spaces always arrives as a single argument on every OS. history of letting a crafted value break out (CVE-2024-3566). Call the interpreter and pass the script as an argument (`python upload.py`, `pwsh -File upload.ps1`) instead. -Scripts are fire and forget. unshackle starts the process, writes the command to the debug -log, and carries on. It does not wait for the script, capture its output, time it out, or -change its own exit code when a script fails. A script keeps running if unshackle exits -first, and Ctrl+C during a season pack does not kill the scripts -already started. +By default, scripts are fire and forget. unshackle starts the process, writes the command to +the debug log, and carries on. It does not wait for the script, capture its output, time it +out, or change its own exit code when a script fails. A script keeps running if unshackle +exits first, and Ctrl+C during a season pack does not kill the +scripts already started. !!! note "One process per file" In `file` mode a large season pack starts one process per episode with nothing - throttling them. If your script is heavy, queue the work inside it or use `mode: season`. + throttling them. If your script is heavy, set `wait: true`, queue the work inside the + script, or use `mode: season`. + +### Waiting for a script { #wait } + +```yaml title="unshackle.yaml" +post_scripts: + - command: "python /opt/upload.py --file={filepath}" + wait: true +``` + +With `wait: true`, unshackle waits until the script exits, then writes the exit code to the +debug log and carries on. Because the `file` hook runs between titles, unshackle runs one +script at a time over a season pack instead of starting them all at once. unshackle only +logs the exit code: a failing script does not fail the download or change unshackle's own +exit code. `--postscript` entries never wait. ## Variables @@ -245,9 +261,10 @@ title object already knew: `{title}`, `{title_raw}`, `{title_id}`, `{year}`, `{s comes from the naming context, such as `{quality}`, `{hdr}`, `{artist}` and `{album}`, is empty. -A resumed `--continue-downloads` download that finishes runs `success` as usual. Because -unshackle does not wait for your script, it never sees its exit code, so a failing script -is not reported anywhere. Your script owns its own error handling and logging. +A resumed `--continue-downloads` download that finishes runs `success` as usual. unshackle +sees your script's exit code only when that entry sets `wait: true`, and even then it only +writes the code to the debug log. Nothing else reports a failing script. Your script owns +its own error handling and logging. A `success` hook always names a file this run wrote. There is no skip-if-exists path in `dl` today, so nothing fires for a title that was not downloaded. diff --git a/tests/test_post_scripts.py b/tests/test_post_scripts.py index 94a15cbd..2aa0c495 100644 --- a/tests/test_post_scripts.py +++ b/tests/test_post_scripts.py @@ -154,7 +154,7 @@ def test_dispatch_survives_an_unbalanced_quote(caplog): ) def test_malformed_post_scripts_config_shapes(monkeypatch, raw, expected): monkeypatch.setattr(config, "post_scripts", raw) - assert list(_entries("success", "file")) == expected + assert [c for c, _ in _entries("success", "file")] == expected def test_typoed_event_or_mode_warns_instead_of_silently_dropping(monkeypatch, caplog): @@ -165,7 +165,7 @@ def test_typoed_event_or_mode_warns_instead_of_silently_dropping(monkeypatch, ca [{"command": "typo.py", "mode": "seson"}, {"command": "ok.py", "mode": "season"}], ) for dispatch_mode in ("file", "season", "run"): - assert list(_entries("success", dispatch_mode)) == (["ok.py"] if dispatch_mode == "season" else []) + assert [c for c, _ in _entries("success", dispatch_mode)] == (["ok.py"] if dispatch_mode == "season" else []) assert caplog.text.count("unknown event/mode") == 1 # warned once, not per dispatch @@ -196,3 +196,24 @@ def test_tagging_ids_are_read_at_dispatch_not_snapshotted(): if __name__ == "__main__": raise SystemExit(pytest.main([__file__, "-q"])) + + +def _dispatch_seconds(monkeypatch, entry: dict) -> float: + import time + + monkeypatch.setattr(config, "post_scripts", [entry]) + start = time.monotonic() + dispatch("success", "file", {}) + return time.monotonic() - start + + +def test_wait_true_waits_for_the_script_and_logs_the_exit_code(monkeypatch, caplog): + caplog.set_level("DEBUG", logger="post-script") + slow = 'python -c "import time,sys; time.sleep(0.3); sys.exit(3)"' + assert _dispatch_seconds(monkeypatch, {"command": slow, "wait": True}) >= 0.3 + assert "Post-script exited 3" in caplog.text + + +def test_wait_defaults_to_fire_and_forget(monkeypatch): + slow = 'python -c "import time; time.sleep(0.3)"' + assert _dispatch_seconds(monkeypatch, {"command": slow}) < 0.3 diff --git a/unshackle/core/utils/post_scripts.py b/unshackle/core/utils/post_scripts.py index 20757238..2af01834 100644 --- a/unshackle/core/utils/post_scripts.py +++ b/unshackle/core/utils/post_scripts.py @@ -6,8 +6,10 @@ is what makes a title like ``Bob"; rm -rf ~`` harmless: a value can never become token or a shell operator. The cost is that the command has no shell features and no interpreter lookup, so users name the interpreter themselves (``python upload.py``). -Scripts are fire-and-forget: unshackle spawns them, logs the command, and moves on. It -does not wait, capture output, time them out, or let a script's exit code change its own. +Scripts are fire-and-forget by default: unshackle spawns them, logs the command, and moves +on. With ``wait: true`` unshackle waits for the script to exit, which runs heavy scripts one +at a time over a season pack. It logs the exit code, but a script never changes unshackle's +own exit code, and unshackle never captures output or times a script out. """ from __future__ import annotations @@ -151,11 +153,11 @@ def season_context(context: dict[str, str], folder: Path) -> dict[str, str]: return out | {"folder": str(folder)} -def _entries(event: str, mode: str, overrides: Sequence[str] = ()) -> Iterator[str]: - """Commands configured for this event and mode. --postscript replaces the config list.""" +def _entries(event: str, mode: str, overrides: Sequence[str] = ()) -> Iterator[tuple[str, bool]]: + """``(command, wait)`` pairs for this event and mode. --postscript replaces the config list.""" if overrides: if event == "success" and mode == "file": - yield from overrides + yield from ((command, False) for command in overrides) return raw = config.post_scripts @@ -189,12 +191,12 @@ def _entries(event: str, mode: str, overrides: Sequence[str] = ()) -> Iterator[s continue if entry_event != event or entry_mode != mode: continue - yield command + yield command, bool(entry.get("wait", False)) def dispatch(event: str, mode: str, context: dict[str, str], overrides: Sequence[str] = ()) -> None: """Spawn every post-script configured for `event` and `mode`. Never raises.""" - for command in _entries(event, mode, overrides): + for command, wait in _entries(event, mode, overrides): try: tokens = tokenize(command) except ValueError as e: # shlex: unbalanced quote in the command template @@ -224,6 +226,15 @@ def dispatch(event: str, mode: str, context: dict[str, str], overrides: Sequence else {"start_new_session": True} ) try: - subprocess.Popen(argv, stdin=subprocess.DEVNULL, **detach) + proc = subprocess.Popen(argv, stdin=subprocess.DEVNULL, **detach) except (OSError, ValueError) as e: log.warning("Post-script failed to start (%s): %s", e, redact_text(redact_path(argv[0]))) + continue + if wait: + rc = proc.wait() + log.debug("Post-script exited %s: %s", rc, safe_command) + log_event( + "post_script_exit", + message=f"Post-script for {event}/{mode} exited {rc}", + context={"event": event, "mode": mode, "command": safe_command, "returncode": rc}, + )