Repository navigation
feat: add 'system' yt-dlp source (PR2 for #182) - #186
Conversation
Third value of ytdlp_source: the addon downloads nothing and only asks whether yt_dlp is importable. Where the package lives is a packaging concern, resolved the same way deno_manager finds a system binary (shutil.which, no path setting). - is_managed_source() / resolve_system_ytdlp() in ytdlp_manager - early return in ensure_ytdlp_ready() and get_runtime_status() - explicit error notification when 'system' is chosen with nothing importable (the entry point already refuses to resolve in that case) - settings.xml option + strings.po label + README note on the importable-dependencies requirement 9 new tests, 294 passed.
When ytdlp_source is 'system' but no yt_dlp package is importable, revert the setting to 'stable' and notify the user. This avoids a silent failure and ensures the addon continues to work. Also fix list_available_versions to return [] for system source instead of raising KeyError.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #186 +/- ##
==========================================
+ Coverage 72.79% 74.69% +1.89%
==========================================
Files 14 14
Lines 2334 2363 +29
==========================================
+ Hits 1699 1765 +66
+ Misses 635 598 -37 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Add 2 tests for configure_managed_ytdlp fallback behavior: - test_system_source_fallback_reverts_to_stable: verifies setSetting, notification, and log when system has no importable yt_dlp - test_system_source_no_fallback_when_importable: verifies no fallback when yt_dlp is importable Also fix 2 log() calls missing the level argument (LOGINFO). 296 tests pass.
|
Thanks a lot, merged! 🎉 Really appreciate the quick follow-up and that you tested it on the NixOS box. I pushed a small follow-up (75c5680) on top:
Would be great if you could give it a quick spin on your appliance with |
|
Gave it the spin you asked for — both directions. On the appliance (LibreELEC, Kodi 22.0-RC1, master incl. 75c5680), with No "reverting to stable" anywhere in the logs. On the NixOS side (importable case): kodi-21.3 with yt-dlp's site-packages pinned Also verified the nightly channel still works with both PRs merged — details in #185. Thanks again for both merges and the follow-up fix — both are working well here. |
Context
Follow-up to #182 and #185 (merged). Adds
systemas a third value ofytdlp_source.What it does
systemuses a yt-dlp that is already importable by Kodi's Python. The addon downloads nothing — it only asks whetheryt_dlpcan be imported. Where the package lives is a packaging concern, resolved the same waydeno_managerfinds a system binary (shutil.which, no path setting).Design decisions
systemdoesn't detect the library, that's a packaging problem — the package and its dependencies must be put where Kodi's Python can import them. The addon has no business knowing or configuring filesystem paths. This is a deliberate trade-off against the single-enum approach: a path setting would be a second configuration axis, but the alternative (silent failure) is worse.systemis chosen but noyt_dlpis importable, the addon revertsytdlp_sourcetostableand shows a notification. This avoids a silent failure and ensures the addon continues to work.autodefault. On most platforms (LibreELEC, Windows, Android) nothing is importable, so silently skipping the download would be the wrong default.systemis an explicit choice.Changes
core/ytdlp_manager.py:YTDLP_SOURCE_SYSTEM,is_managed_source(),resolve_system_ytdlp(), early returns inensure_ytdlp_ready()andget_runtime_status(), guard inlist_available_versions()core/runtime/actions.py: fallback to stable with notification whensystemhas no importable libraryresources/settings.xml: third optionsystemresources/language/.../strings.po: label#32072README.md: documents the importable-dependencies requirementtests/test_ytdlp_system_source.py: 9 new testsTesting
yt_dlpfrom Nix store detected,ready=True,runtime_path=NoneNotes
413281c3(firsttris's tidy-up of Add yt-dlp source selection: stable / nightly #185)