mirror of
https://github.com/unshackle-dl/unshackle.git
synced 2026-11-12 12:05:07 +00:00
fix(api): survive Windows errno 13 on the job progress file
This commit is contained in:
@@ -7,6 +7,7 @@ cleanup/serialize); they do not exercise the actual subprocess download path.
|
||||
from __future__ import annotations
|
||||
|
||||
from datetime import datetime, timedelta
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
@@ -141,3 +142,30 @@ def test_normalize_sub_format_preserves_original_sentinel() -> None:
|
||||
# A real codec still maps to its enum member; unknowns collapse to None.
|
||||
assert normalize_sub_format("srt") is Subtitle.Codec.SubRip
|
||||
assert normalize_sub_format("nonsense") is None
|
||||
|
||||
|
||||
def test_worker_write_result_retries_denied_replace(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Windows denies os.replace while the parent holds the progress file open, so the write must retry.
|
||||
|
||||
Without the retry a denied replace loses that update. A value the worker writes once, such as a
|
||||
phase change, then stays invisible until the next write.
|
||||
"""
|
||||
import json
|
||||
import os
|
||||
|
||||
from unshackle.core.api import download_worker
|
||||
|
||||
target = tmp_path / "progress.json"
|
||||
real_replace = os.replace
|
||||
attempts = []
|
||||
|
||||
def flaky_replace(src: Path, dst: Path) -> None:
|
||||
attempts.append(dst)
|
||||
if len(attempts) == 1:
|
||||
raise PermissionError(13, "Permission denied", str(dst))
|
||||
real_replace(src, dst)
|
||||
|
||||
monkeypatch.setattr(download_worker.os, "replace", flaky_replace)
|
||||
download_worker.write_result(target, {"progress": 42.0})
|
||||
assert json.loads(target.read_text(encoding="utf-8")) == {"progress": 42.0}
|
||||
assert len(attempts) == 2
|
||||
|
||||
@@ -1004,7 +1004,6 @@ class DownloadQueueManager:
|
||||
stat = os.stat(progress_path)
|
||||
stat_key = (stat.st_mtime_ns, stat.st_size)
|
||||
if stat_key != last_progress_stat:
|
||||
last_progress_stat = stat_key
|
||||
with open(progress_path, "r", encoding="utf-8") as handle:
|
||||
progress_data = json.load(handle)
|
||||
if progress_data.get("phase") and progress_data["phase"] != job.phase:
|
||||
@@ -1041,7 +1040,8 @@ class DownloadQueueManager:
|
||||
if progress_data != last_published:
|
||||
last_published = progress_data
|
||||
self.publish(job, "progress")
|
||||
except (FileNotFoundError, json.JSONDecodeError, ValueError) as e:
|
||||
last_progress_stat = stat_key
|
||||
except (OSError, json.JSONDecodeError, ValueError) as e:
|
||||
log.debug(f"Could not read progress for job {job.job_id}: {e}")
|
||||
|
||||
if job.cancel_event.is_set() or job.status == JobStatus.CANCELLED:
|
||||
|
||||
@@ -6,6 +6,7 @@ import json
|
||||
import logging
|
||||
import os
|
||||
import sys
|
||||
import time
|
||||
import traceback
|
||||
from pathlib import Path
|
||||
from typing import Any, Dict
|
||||
@@ -21,10 +22,19 @@ def read_payload(path: Path) -> Dict[str, Any]:
|
||||
|
||||
|
||||
def write_result(path: Path, payload: Dict[str, Any]) -> None:
|
||||
# Atomic replace: the parent polls this file while it is being rewritten.
|
||||
"""Write the payload with an atomic replace, because the parent polls this file during the write.
|
||||
|
||||
Windows denies the replace while the parent holds the destination open, so retry with a backoff.
|
||||
"""
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
tmp = path.with_name(path.name + ".tmp")
|
||||
tmp.write_text(json.dumps(payload), encoding="utf-8")
|
||||
for attempt in range(5):
|
||||
try:
|
||||
os.replace(tmp, path)
|
||||
return
|
||||
except PermissionError:
|
||||
time.sleep(0.02 * (attempt + 1))
|
||||
os.replace(tmp, path)
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user