danvics/soulsync
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

youtube download: don't clobber target_dir → fixes double-nested folders

Browse source 
bug: process_youtube_download wrote the ORGANISED dir (channel/Season YYYY) back
to the row's target_dir, but target_dir is supposed to be the youtube ROOT — and
plan_destination re-derives channel/season UNDER it. so any re-processing re-nested:
Channel/Season 2026/Channel/Season 2026/. that hit the 1-2 videos per channel that
got interrupted mid-download and re-queued by the orphan reaper.

fix: only record the organised filename for display; never write target_dir. the
root stays put so re-runs are idempotent. regression test runs it twice + asserts
no target_dir clobber + a stable (non-nested) dest dir.
This commit is contained in:
BoulderBadgeDad 2026-06-26 08:37:47 -07:00
parent f0e1fda0fc
commit fc045cd74e
2 changed files with 29 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
core/video/youtube_download.py
View file

@ -162,9 +162,13 @@ def process_youtube_download(
dest = plan_destination(dl, settings, container)
stem, cont = _stem_and_container(dest, container)
# Record where it's going up front (Downloads page shows the organised name).
# Record the organised filename for the Downloads page. CRITICAL: do NOT write the
# organised DIR back to target_dir — target_dir is the youtube ROOT, and plan_destination
# re-derives the channel/season folders under it. Clobbering it would re-nest on any
# re-run (e.g. the orphan reaper re-queues an interrupted download) →
# Channel/Season/Channel/Season. Leave target_dir = root so re-processing is idempotent.
update_row(dl.get("id"), status="downloading", progress=0,
target_dir=dest.get("dir"), filename=dest.get("filename"))
filename=dest.get("filename"))
res = download(dl.get("media_id"), dest.get("dir"), stem, profile, cont,
progress_hook=progress_hook, cookie_opts=cookie_opts)

23
tests/test_youtube_download.py
View file

@ -179,3 +179,26 @@ def test_process_passes_the_organised_dir_to_the_downloader():
assert seen["video_id"] == "vid1"
assert seen["dest_dir"] == os.path.join("/yt", "Chan", "Season 2024")
assert seen["stem"] == "Chan - 2024-03-15 - T" and seen["container"] == "mp4"
def test_process_never_clobbers_target_dir_so_reruns_dont_nest():
"""The row's target_dir is the youtube ROOT; plan_destination derives the channel/season
folders under it. The worker must NOT write the organised dir back to target_dir, or a
re-run (e.g. the orphan reaper re-queues an interrupted download) would organise AGAIN →
Channel/Season/Channel/Season. Re-processing the same row must be idempotent."""
seen = []
def fake_download(video_id, dest_dir, stem, profile, container, **kw):
seen.append(dest_dir)
return {"ok": True, "dest_path": "/x"}
calls, update_row, archive, clear = _recorder()
dl = _dl() # target_dir = "/yt" (the root)
for _ in range(2): # simulate the interrupted-then-requeued re-run
ytd.process_youtube_download(dl, profile=default_profile(), settings={},
download=fake_download, update_row=update_row,
archive=archive, clear_wishlist=clear, now=lambda: "t")
# no update_row call writes target_dir (that's what caused the nesting)
assert all("target_dir" not in kw for _, kw in calls["rows"])
# both runs target the SAME organised dir — not a doubly-nested one
assert seen[0] == seen[1] == os.path.join("/yt", "Chan", "Season 2024")