video downloads Phase A: cancelled state, cancel/retry, monitor robustness
Fixes the 'cancelled but still shows running' stuck bug and adds real depth: - classify_state now distinguishes 'cancelled' from 'failed'. - Monitor is robust to slskd forgetting a transfer: if it's gone, it first tries to complete from the FILE on disk (survives the music 'Clean Completed Downloads' auto-clear), else counts misses and fails the row after ~8 polls instead of hanging on 'downloading' forever. Cancelled transfers → cancelled status. - POST /downloads/cancel (slskd DELETE transfer + mark cancelled) and /downloads/retry (re-grab the same release). get_video_download(id) added; clear includes cancelled. process_download stays pure (fs/slskd injected); 15 tests, ruff + guard clean. Phase B (page: tabs/queue/history/cancel+retry buttons) next.
This commit is contained in:
parent
42c67a6d4d
commit
0bb77bf782
5 changed files with 147 additions and 21 deletions
|
|
@ -276,6 +276,45 @@ def register_routes(bp):
|
|||
ensure_started(get_video_db) # also (re)start the monitor when the page is open
|
||||
return jsonify({"downloads": db.list_video_downloads()})
|
||||
|
||||
@bp.route("/downloads/cancel", methods=["POST"])
|
||||
def video_downloads_cancel():
|
||||
from . import get_video_db
|
||||
from core.video.slskd_download import cancel_download
|
||||
body = request.get_json(silent=True) or {}
|
||||
db = get_video_db()
|
||||
dl = db.get_video_download(body.get("id"))
|
||||
if not dl:
|
||||
return jsonify({"ok": False, "error": "Download not found."}), 404
|
||||
if dl["status"] in ("completed", "failed", "cancelled"):
|
||||
return jsonify({"ok": True, "already": True})
|
||||
cancel_download(dl.get("username"), dl.get("filename")) # best-effort; mark regardless
|
||||
import time
|
||||
db.update_video_download(dl["id"], status="cancelled", error="Cancelled",
|
||||
completed_at=time.strftime("%Y-%m-%d %H:%M:%S"))
|
||||
return jsonify({"ok": True})
|
||||
|
||||
@bp.route("/downloads/retry", methods=["POST"])
|
||||
def video_downloads_retry():
|
||||
"""Re-grab the SAME release (basic retry). Auto-retry + alternate-query retry
|
||||
come in a later phase."""
|
||||
from . import get_video_db
|
||||
from core.video.download_monitor import ensure_started
|
||||
from core.video.slskd_download import start_download
|
||||
body = request.get_json(silent=True) or {}
|
||||
db = get_video_db()
|
||||
dl = db.get_video_download(body.get("id"))
|
||||
if not dl:
|
||||
return jsonify({"ok": False, "error": "Download not found."}), 404
|
||||
if not dl.get("username") or not dl.get("filename"):
|
||||
return jsonify({"ok": False, "error": "Nothing to retry from."}), 400
|
||||
started = start_download(dl["username"], dl["filename"], dl.get("size_bytes") or 0)
|
||||
if not started.get("ok"):
|
||||
return jsonify({"ok": False, "error": started.get("error") or "slskd refused the download."}), 502
|
||||
db.update_video_download(dl["id"], status="downloading", progress=0, error=None,
|
||||
dest_path=None, completed_at=None)
|
||||
ensure_started(get_video_db)
|
||||
return jsonify({"ok": True})
|
||||
|
||||
@bp.route("/downloads/clear", methods=["POST"])
|
||||
def video_downloads_clear():
|
||||
from . import get_video_db
|
||||
|
|
|
|||
|
|
@ -30,21 +30,12 @@ _started = False
|
|||
_lock = threading.Lock()
|
||||
|
||||
|
||||
def process_download(dl: dict, transfers: list, download_dir: str, *, lister, mover) -> dict | None:
|
||||
"""Decide the next state for one active download given the current slskd transfers.
|
||||
Returns a patch dict for the DB row (or None to leave it untouched this tick)."""
|
||||
t = find_transfer(transfers, dl.get("username"), dl.get("filename"))
|
||||
if not t:
|
||||
return None # not registered yet (or cleared) — wait
|
||||
state = classify_state(t.get("state"))
|
||||
if state == "active":
|
||||
return {"status": "downloading", "progress": progress_pct(t)}
|
||||
if state == "failed":
|
||||
return {"status": "failed", "error": "Soulseek transfer " + str(t.get("state") or "failed")}
|
||||
# completed → locate the file on disk and move it into the library folder
|
||||
def _complete_via_file(dl, download_dir, lister, mover):
|
||||
"""Locate the finished file in the download dir and move it to the library. Returns
|
||||
a completed/failed patch, or {'progress':100} if the file isn't there yet."""
|
||||
src = find_completed_file(download_dir, dl.get("filename"), lister)
|
||||
if not src:
|
||||
return {"progress": 100.0} # slskd done; file still settling — retry
|
||||
return {"progress": 100.0}
|
||||
dest = dest_path_for(dl.get("target_dir"), src)
|
||||
try:
|
||||
mover(src, dest)
|
||||
|
|
@ -53,6 +44,29 @@ def process_download(dl: dict, transfers: list, download_dir: str, *, lister, mo
|
|||
return {"status": "completed", "progress": 100.0, "dest_path": dest}
|
||||
|
||||
|
||||
def process_download(dl: dict, transfers: list, download_dir: str, *, lister, mover) -> dict | None:
|
||||
"""Decide the next state for one active download given the current slskd transfers.
|
||||
Returns a patch dict for the DB row, or {'_missing': True} when slskd no longer
|
||||
knows the transfer (the caller decides when to give up). Robust to slskd clearing
|
||||
completed transfers (the music 'Clean Completed Downloads' automation) by also
|
||||
detecting completion from the file landing on disk."""
|
||||
t = find_transfer(transfers, dl.get("username"), dl.get("filename"))
|
||||
if not t:
|
||||
# slskd forgot it — could be done+cleared. If the file's there, finish it.
|
||||
done = _complete_via_file(dl, download_dir, lister, mover)
|
||||
if done.get("status"):
|
||||
return done
|
||||
return {"_missing": True}
|
||||
state = classify_state(t.get("state"))
|
||||
if state == "active":
|
||||
return {"status": "downloading", "progress": progress_pct(t)}
|
||||
if state == "cancelled":
|
||||
return {"status": "cancelled", "error": "Cancelled on Soulseek"}
|
||||
if state == "failed":
|
||||
return {"status": "failed", "error": "Soulseek transfer " + str(t.get("state") or "failed")}
|
||||
return _complete_via_file(dl, download_dir, lister, mover) # completed
|
||||
|
||||
|
||||
def _move(src: str, dest: str) -> None:
|
||||
os.makedirs(os.path.dirname(dest) or ".", exist_ok=True)
|
||||
shutil.move(src, dest)
|
||||
|
|
@ -64,23 +78,46 @@ def _walk(root: str):
|
|||
yield os.path.join(dirpath, f)
|
||||
|
||||
|
||||
_GIVE_UP_AFTER = 8 # consecutive 'transfer gone, no file' polls before failing it
|
||||
_misses: dict = {} # download id -> consecutive missing polls
|
||||
|
||||
|
||||
def _tick(db) -> None:
|
||||
active = db.get_active_video_downloads()
|
||||
if not active:
|
||||
_misses.clear()
|
||||
return
|
||||
from config.settings import config_manager
|
||||
download_dir = str(config_manager.get("soulseek.download_path", "") or "")
|
||||
transfers = list_downloads()
|
||||
live_ids = set()
|
||||
for dl in active:
|
||||
live_ids.add(dl["id"])
|
||||
upd = process_download(dl, transfers, download_dir, lister=_walk, mover=_move)
|
||||
if not upd:
|
||||
continue
|
||||
if upd.get("status") in ("completed", "failed"):
|
||||
if upd.get("_missing"):
|
||||
# slskd no longer has the transfer and the file never appeared. Give it a
|
||||
# few polls (a just-cancelled transfer vanishes), then mark it failed so it
|
||||
# doesn't sit on 'downloading' forever.
|
||||
n = _misses.get(dl["id"], 0) + 1
|
||||
_misses[dl["id"]] = n
|
||||
if n >= _GIVE_UP_AFTER:
|
||||
_misses.pop(dl["id"], None)
|
||||
db.update_video_download(dl["id"], status="failed",
|
||||
error="Soulseek transfer disappeared",
|
||||
completed_at=time.strftime("%Y-%m-%d %H:%M:%S"))
|
||||
continue
|
||||
_misses.pop(dl["id"], None)
|
||||
if upd.get("status") in ("completed", "failed", "cancelled"):
|
||||
upd.setdefault("completed_at", time.strftime("%Y-%m-%d %H:%M:%S"))
|
||||
try:
|
||||
db.update_video_download(dl["id"], **upd)
|
||||
except Exception:
|
||||
logger.exception("video download %s: failed to persist update", dl.get("id"))
|
||||
# drop miss counters for ids that are no longer active
|
||||
for k in [k for k in _misses if k not in live_ids]:
|
||||
_misses.pop(k, None)
|
||||
|
||||
|
||||
def _run(db_provider) -> None:
|
||||
|
|
|
|||
|
|
@ -71,17 +71,40 @@ def flatten_downloads(data: Any) -> list:
|
|||
|
||||
|
||||
def classify_state(state: Any) -> str:
|
||||
"""slskd state string → 'completed' | 'failed' | 'active'. Pure."""
|
||||
"""slskd state string → 'completed' | 'cancelled' | 'failed' | 'active'. Pure."""
|
||||
s = str(state or "").lower()
|
||||
if "completed" in s and "succeed" in s:
|
||||
return "completed"
|
||||
if any(x in s for x in ("error", "cancel", "timed", "failed", "reject")):
|
||||
if "cancel" in s:
|
||||
return "cancelled"
|
||||
if any(x in s for x in ("error", "timed", "failed", "reject")):
|
||||
return "failed"
|
||||
if "completed" in s: # completed but not succeeded → treat as failed
|
||||
return "failed"
|
||||
return "active"
|
||||
|
||||
|
||||
def cancel_download(username: str, filename: str) -> dict:
|
||||
"""Cancel (and remove) a slskd transfer matching username+filename. Returns
|
||||
{ok[, gone][, error]}. 'gone' = the transfer was already absent."""
|
||||
base, headers = _conn()
|
||||
if not base:
|
||||
return {"ok": False, "error": "slskd isn't configured"}
|
||||
tid = None
|
||||
for t in list_downloads():
|
||||
if t.get("username") == username and t.get("filename") == filename:
|
||||
tid = t.get("id")
|
||||
break
|
||||
if not tid:
|
||||
return {"ok": True, "gone": True}
|
||||
try:
|
||||
requests.delete(base + "/api/v0/transfers/downloads/%s/%s" % (quote(str(username or "")), tid),
|
||||
headers=headers, params={"remove": "true"}, timeout=10)
|
||||
return {"ok": True}
|
||||
except Exception as e: # noqa: BLE001 - surface the failure; the row is marked cancelled anyway
|
||||
return {"ok": False, "error": str(e)}
|
||||
|
||||
|
||||
def progress_pct(transfer: dict) -> float:
|
||||
"""0–100 from bytesTransferred/size (100 once completed). Pure."""
|
||||
transfer = transfer or {}
|
||||
|
|
@ -101,4 +124,4 @@ def find_transfer(transfers: list, username: str, filename: str) -> dict:
|
|||
|
||||
|
||||
__all__ = ["start_download", "list_downloads", "flatten_downloads", "classify_state",
|
||||
"progress_pct", "find_transfer"]
|
||||
"progress_pct", "find_transfer", "cancel_download"]
|
||||
|
|
|
|||
|
|
@ -1158,6 +1158,14 @@ class VideoDatabase:
|
|||
finally:
|
||||
conn.close()
|
||||
|
||||
def get_video_download(self, dl_id: int) -> dict | None:
|
||||
conn = self._get_connection()
|
||||
try:
|
||||
row = conn.execute("SELECT * FROM video_downloads WHERE id = ?", (int(dl_id),)).fetchone()
|
||||
return dict(row) if row else None
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
def get_active_video_downloads(self) -> list:
|
||||
conn = self._get_connection()
|
||||
try:
|
||||
|
|
@ -1185,7 +1193,7 @@ class VideoDatabase:
|
|||
def clear_finished_video_downloads(self) -> int:
|
||||
conn = self._get_connection()
|
||||
try:
|
||||
cur = conn.execute("DELETE FROM video_downloads WHERE status IN ('completed', 'failed')")
|
||||
cur = conn.execute("DELETE FROM video_downloads WHERE status IN ('completed', 'failed', 'cancelled')")
|
||||
conn.commit()
|
||||
return cur.rowcount
|
||||
finally:
|
||||
|
|
|
|||
|
|
@ -22,7 +22,7 @@ def test_classify_state():
|
|||
assert classify_state("InProgress") == "active"
|
||||
assert classify_state("Queued, Remotely") == "active"
|
||||
assert classify_state("Completed, Errored") == "failed"
|
||||
assert classify_state("Completed, Cancelled") == "failed"
|
||||
assert classify_state("Completed, Cancelled") == "cancelled"
|
||||
assert classify_state("Completed, TimedOut") == "failed"
|
||||
assert classify_state("") == "active"
|
||||
|
||||
|
|
@ -112,9 +112,28 @@ def test_process_download_failed():
|
|||
assert upd["status"] == "failed"
|
||||
|
||||
|
||||
def test_process_download_no_transfer_yet():
|
||||
def test_process_download_cancelled():
|
||||
from core.video.download_monitor import process_download
|
||||
assert process_download(_dl(), [], "/dl", lister=lambda d: [], mover=lambda s, d: None) is None
|
||||
upd = process_download(_dl(), [_xfer("Completed, Cancelled")], "/dl",
|
||||
lister=lambda d: [], mover=lambda s, d: None)
|
||||
assert upd["status"] == "cancelled"
|
||||
|
||||
|
||||
def test_process_download_missing_transfer_signals_missing():
|
||||
from core.video.download_monitor import process_download
|
||||
# slskd forgot it AND no file on disk → _missing (caller decides when to give up)
|
||||
upd = process_download(_dl(), [], "/dl", lister=lambda d: [], mover=lambda s, d: None)
|
||||
assert upd == {"_missing": True}
|
||||
|
||||
|
||||
def test_process_download_missing_but_file_present_completes():
|
||||
from core.video.download_monitor import process_download
|
||||
moved = {}
|
||||
# slskd cleared the completed transfer (the music auto-clear) but the file is there
|
||||
upd = process_download(_dl(), [], "/dl",
|
||||
lister=lambda d: ["/dl/Folder/movie.mkv"],
|
||||
mover=lambda s, d: moved.update(ok=True))
|
||||
assert upd["status"] == "completed" and moved.get("ok") is True
|
||||
|
||||
|
||||
def test_process_download_completed_moves_file():
|
||||
|
|
|
|||
Loading…
Reference in a new issue