fix(podcasts): guard stream when audio missing and share object store volume#1500
Conversation
|
@CREDO23 is attempting to deploy a commit to the Rohan Verma's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughA persistent Docker volume ( ChangesPodcast streaming bug fix
Sequence DiagramsequenceDiagram
participant Client
participant stream_podcast
participant audio_exists
participant StorageBackend
Client->>stream_podcast: GET /podcasts/{id}/stream
stream_podcast->>audio_exists: audio_exists(podcast)
audio_exists->>StorageBackend: exists(storage_key)
StorageBackend-->>audio_exists: bool
alt storage object missing
audio_exists-->>stream_podcast: False
stream_podcast-->>Client: 404 Podcast audio not found
else podcast status is non-terminal / in-flight
stream_podcast-->>Client: 409 Podcast audio is not ready yet
else audio present
stream_podcast->>StorageBackend: open_stream(storage_key)
StorageBackend-->>stream_podcast: audio bytes
stream_podcast-->>Client: 200 StreamingResponse
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
87e6026 to
a7be41d
Compare
Summary
/streamendpoints withaudio_exists()so a missing object returns 404/409 instead of an unhandledFileNotFoundErrormid-stream (fixes [BUG] Podcast streaming fails when MP3 file is missing #1498).object_storevolume onbackendandcelery_workerso the default local storage backend works across split containers.Test plan
pytest tests/integration/podcasts/test_streaming.py tests/integration/podcasts/test_public_stream.pypytest tests/integration/podcasts/(45 passed)High-level PR Summary
This PR fixes two podcast streaming issues: it adds guards to the authenticated and public
/streamendpoints to check if audio files exist before streaming (returning 404 for missing objects and 409 for in-flight podcasts instead of crashing with unhandledFileNotFoundError), and it configures a shared persistentobject_storevolume in Docker Compose so that audio files stored by the Celery worker are accessible to the backend API across separate containers.⏱️ Estimated Review Time: 5-15 minutes
💡 Review Order Suggestion
surfsense_backend/app/podcasts/storage.pysurfsense_backend/tests/integration/podcasts/conftest.pysurfsense_backend/app/podcasts/api/routes.pysurfsense_backend/app/routes/public_chat_routes.pysurfsense_backend/tests/integration/podcasts/test_streaming.pysurfsense_backend/tests/integration/podcasts/test_public_stream.pydocker/docker-compose.ymldocker/docker-compose.dev.ymlSummary by CodeRabbit
Bug Fixes
Infrastructure