Add YuTorah provider for streaming Torah shiurim#3429
Add YuTorah provider for streaming Torah shiurim#3429russlevy wants to merge 20 commits intomusic-assistant:devfrom
Conversation
🔒 Dependency Security Report✅ No dependency changes detected in this PR. |
|
Label should be |
|
Also having the whole provider in one file is too much. Please split this into init.py, constants.py, helpers.py and provider.py |
|
There is a bit to consider here. Marking as draft. Set it back to review required when it’s ready. |
Should I mark it as ready when I'm done getting through everything? Just moving functions around right now |
|
As your resolve each comment then mark it as such by clicking the resolve conversation button. Yes once you think it is ready for review towards merging then click the “Ready for Review” button. Just don’t rush it. |
OK great. My general practice has been to make the reviewer resolve when they are happy with the fix, but happy to do it too! Of course not rushing, going to spend a bit of time thinking through the whole podcast / teacher stuff. probably needs a bunch of testing too, when I'm happy with the result I'll mark it as ready for review. |
84c5673 to
58da8cb
Compare
Co-authored-by: marcelveldt <6389780+marcelveldt@users.noreply.github.com>
…sic-assistant#3439) * Clean up party plugin config options Remove album art background config option and make party player selection independent of guest access toggle so party mode works without QR code enabled. Clean up karaoke mode label. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Remove QR code instruction text config options from party plugin Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Parallelize filesystem library sync for faster NFS ingestion Replace the serial single-threaded sync loop with a two-phase approach: - Phase 1: Enumerate files in an executor thread (fast metadata scan) - Phase 2: Process changed files concurrently using TaskManager(16) This replaces blocking parse_tags + run_coroutine_threadsafe(..).result() calls with async_parse_tags, allowing up to 16 ffprobe processes to run concurrently. For NFS mounts where each ffprobe has ~500ms latency, this yields ~16x throughput improvement on tag parsing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Report sync progress percentage to frontend Use update_current_task_progress_from_index during Phase 2 of library sync so the frontend can display a progress bar instead of just text. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: OzGav <gavnosp@hotmail.com>
Co-authored-by: OzGav <gavnosp@hotmail.com>
add YU shield icon for yutorah provider fix: raise SetupFailedError on login failure refactor: split into multiple files, require login fix: remove LIBRARY_PODCASTS and get_library_podcasts override small fixes fix: split some files and pr review comments small logical fixes
russlevy
left a comment
There was a problem hiding this comment.
This should be ready for re-review. I've tried to manually test every path as well too ensure it's working. As well, I felt that provider.py was going to be too big still, so I copied Apple Music and created a browse.py file. Finally, felt some of the functions were a bit unwieldy so broke them up into helper functions.
| ) | ||
|
|
||
|
|
||
| def _shiur_to_track( |
There was a problem hiding this comment.
I don't think returning shiurim as Track objects in search results is right. A shiur has no artist and no album, so the track will appear incomplete in the search UI. More importantly, MA will treat it as a music track and attempt metadata enrichment — MusicBrainz lookups etc. — for content that is fundamentally a podcast episode. If SearchResults has no podcast_episodes field, should we raise a model enhancement request to add one rather than misusing Track?
@MarvinSchenkel thoughts on this?
There was a problem hiding this comment.
Yeah agree with @OzGav here. Looking at this provider, it exclusively delivers podcasts / podcast episodes. I think we can remove any references to Artists and Tracks. Since there are not library_xxxx functions (because I don't think YuToarh has a library?), you can rely on browse endpoints so people can still browse the content by teacher for example.
There was a problem hiding this comment.
The issue is the discoverability. Almost all of the episodes are standalone, and you may want to search for a specific topic to find it across different teachers and series. This is how it works on yutorah.org as well -- though there are teacher, and teacher / series pairs (which we are modeling as podcasts), the primary search is by class / shiur / episode
There was a problem hiding this comment.
Then the fix is possibly to adjust the search to return results which match the teacher name as well as the podcast name. You can also implement different browse folders if there are topics that users might typically be interested in.
|
There are a bunch of tests failing (although one looks unrelated to your code so ignore that) |
| }, | ||
| ) | ||
|
|
||
| async def get_artist_toptracks(self, prov_artist_id: str) -> list[Track]: |
There was a problem hiding this comment.
I’m still not sure about teachers as artists but on this particular method, it won’t get called as you haven’t added the relevant provider feature
|
Please update the PR description with any changes made through this process. This will probably get looked at further next week. |
| teacher_name = shiur.get("teacherfullname") or "" | ||
| photo = shiur.get("PHOTO") or shiur.get("photo") or "" | ||
| if photo and not photo.startswith("http"): | ||
| photo = f"https://cdnyutorah.cachefly.net/_images/roshei_yeshiva/{photo}" |
There was a problem hiding this comment.
We can move this into constants.py too
|
Hey @russlevy, marking this PR as draft so we can keep on top of which PRs need our attention. Just flag it as 'ready for review' when you want us to have another look! |
Adds a new music provider for YuTorah Online, streaming over 400,000 Torah audio lectures via the official mobile app API.
Features:
Data model: