POK-307: Add download progress to medcat - #37
Conversation
saheljalal
left a comment
There was a problem hiding this comment.
Verdict: Approve — correct, well-scoped, and well-tested.
Traced all four transfer paths and the new sk.track_bytes. The change does what it claims: direct HTTP + Internet Archive streams get a real Rich progress bar (%, bytes, speed, ETA, indeterminate fallback), and yt-dlp/ia/wget now inherit the terminal so their native progress is no longer swallowed by capture_output. Removing capture_output also correctly dropped the now-None proc.stderr[:200] references and replaced them with exit-code messages — the real error still streams inline to the user, which is arguably better. Other subprocess.run calls that still parse proc.stdout (metadata/json probes, ingest) were left capturing. Version bumps (scriptkit 1.3.0, medcat 2.4.0), docs, and changelog are consistent.
track_bytes mirrors track's HAS_RICH / console / use_color guards, so the API fit is clean, and the total normalization (max(0, int(total)) or None) sensibly collapses 0/negative to the indeterminate bar. Ran the 6 new tests plus the progress/characterization suites locally — all green.
Optional nits (non-blocking):
-
medcat:3254—total = int(content_length) if content_length.isdigit() else result.size.content_lengthis defensively.isdigit()-checked butresult.sizeis trusted straight intotrack_bytes, which doesint(total). It'sint|Noneby theSearchResultcontract and the direct-probe path (medcat:3201) only ever sets an int/None, so this is safe today — but a stray non-numeric size would raise mid-download and surface as a generic "Download error". A cheap guard (coerce/isinstancecheck) would match the robustness already applied tocontent_length. -
scriptkit/progress.py— the loop yields the chunk beforeprogress.update(..., advance=len(chunk)), so the bar trails the write by one chunk and reaches 100% only after the final chunk is consumed. Purely cosmetic; fine to leave. -
Pre-existing (not introduced here): on a mid-stream connection error the partial
out_pathfile is left on disk. Worth a future cleanup on theexcept, but out of scope for this PR.
Summary
sk.track_bytesprogress with percentage, byte counts, speed, and ETAyt-dlpandiainstead of capturing itTests
../scripts/venv/bin/python -m pytest(431 passed)../scripts/venv/bin/python -m py_compile medcat scriptkit/*.pygit diff --checkCloses POK-307