perf: offload transcript download cleanup to a worker thread - #1246
perf: offload transcript download cleanup to a worker thread#1246groupthinking wants to merge 1 commit into
Conversation
The `finally` block in `_fallback_transcript_with_gemini` deleted the downloaded video and recursively removed its temp tree directly on the event loop. `Path.exists`, `Path.unlink` and `shutil.rmtree` are all blocking syscalls, and the temp tree can hold a merged mp4 plus unmerged `.fNNN` fragments, so every request that reaches the Gemini video fallback stalls the loop for the duration of the delete. Move the cleanup into `_cleanup_download_artifacts`, a static helper that runs the same logic under `asyncio.to_thread`. The call is wrapped in `asyncio.shield` because the original inline code was uncancellable: a bare `await` in a `finally` can be interrupted by a second cancellation, which would turn a loop stall into a disk leak. Filesystem semantics are preserved verbatim, including the `exists()` guards, `except OSError` and `ignore_errors=True`. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
Agent Completion Truth Gate: BLOCKEDReasons: Machine-readable verdict{
"details": {
"invalid_fields": [
"issue.number",
"policy.agent_login",
"policy.run_id"
]
},
"reasons": [
"invalid_payload"
],
"verdict": "blocked"
} |
|
Closing as a duplicate of #1245. This PR points at the identical head commit ( Review is happening on #1245. No work is lost by closing this. |
1 participant
Canonical issue
Closes #
Outcome
The Gemini video transcription fallback no longer stalls the asyncio event loop while it deletes downloaded artifacts. Every request that reaches
_fallback_transcript_with_geminipreviously ranPath.exists/Path.unlink/shutil.rmtree— all blocking syscalls — inline in afinallyblock, over a temp tree that can hold a merged mp4 plus unmerged.fNNNfragments. That unbounded disk work now runs in a worker thread.Scope
src/youtube_extension/services/workflows/transcript_action_workflow.py— extract inline cleanup into a static_cleanup_download_artifactshelper that runs underasyncio.to_thread, wrapped inasyncio.shield.tests/unit/test_transcript_action_workflow.py— newTestCleanupDownloadArtifactsOffEventLoopcoverage.Risk
finallycould previously skip an unshielded await and leak the temp tree;asyncio.shieldpreserves the original "always cleans up" property while still yielding the loop and re-raisingCancelledError.except OSError,ignore_errors=Trueare unchanged).Verification
None/missing paths, and runsrmtreeon a non-event-loop thread. The committedpytestsuite (TestCleanupDownloadArtifactsOffEventLoop) asserts the same viamonkeypatch, including a non-vacuous guard that both primitives actually ran and a concurrency test proving the loop keeps running during deletion.pytestsuite in the sandbox (backend runtime deps such ashttpx/pydanticare not installed); relying on repo CI on the current head.Production evidence
Python-only change to backend workflow cleanup; not exercised by the Next.js Vercel preview. Behavior is identical to the prior synchronous path apart from where the deletion runs, so no new runtime surface is introduced.
Agent handoff
Agent provenance
Underlying commit is human-authored (
Hayden, co-authored by Copilot App); provenance/agent-lock finalization is left to the maintainer. Kept as draft pending CI and human review.Generated by Claude Code