fix: handle OLAK chart playlists without album info #854
Conversation
Some OLAK playlists (like charts/trending playlists) contain music videos without album info. Previously, parse_audio_playlist() would crash trying to access album["name"] on tracks without album info. Changes: - parse_audio_playlist() now returns None if not all tracks have album info, signaling to the caller to fall back to regular playlist parsing - Simplified the routing logic in get_playlist() to check return value - Added test case for chart playlist (OLAK5uy_mzYnlaHgFOvLaxqIPnnouEr-idiUn4NIM) Fixes TypeError: 'NoneType' object is not subscriptable when fetching chart playlists like "Trending 20 Spain".
|
Hi @sigma67, The CI is failing due to two test failures that appear to be unrelated to the changes in this PR:
Both tests hit the live YouTube API and are susceptible to changes in YouTube's responses. The coverage report also failed to generate because pytest exited before The actual changes in this PR (handling OLAK chart playlists without album info) are tested by the new mock-based test case which passed successfully. Would you be able to re-run the CI or let me know if there's anything I should adjust on my end? |
|
Thanks, I will have a look later |
|
Here's another OLAK playlist that is not an album: https://music.youtube.com/playlist?list=OLAK5uy_meFrWp55M2SJ6yzAtKBPF1Uq_viHSihmE |
|
Could you please merge main to check if the tests now pass. I cannot edit this PR |
| def parse_audio_playlist( | ||
| response: JsonDict, limit: int | None, request_func: RequestFuncBodyType | ||
| ) -> JsonDict: | ||
| ) -> JsonDict | None: |
There was a problem hiding this comment.
Please don't change this function
| request_func_continuations: RequestFuncBodyType = lambda body: self._send_request(endpoint, body) | ||
| if playlistId.startswith("OLA") or playlistId.startswith("VLOLA"): | ||
| return parse_audio_playlist(response, limit, request_func_continuations) | ||
| # Try parsing as audio/album playlist - returns None if not all tracks have album info |
There was a problem hiding this comment.
Honestly it's a bit strange how you solved this imo. You need a better way to determine if the playlist is affected (structurally missing album info).
Wouldn't it be simpler to simply not skip this branch for OLAK playlists by updating the if condition?
Checking for the absence of some parsed items in the result of the parser is exceptionally fragile and could have any number of other causes (empty playlist for example)
2 participants
Fixes #853
Summary
Some OLAK playlists (like charts/trending playlists) contain music videos without album info. Previously,
parse_audio_playlist()would crash trying to accessalbum["name"]on tracks without album info.Example:
OLAK5uy_mzYnlaHgFOvLaxqIPnnouEr-idiUn4NIM("Trending 20 Spain") contains OMVs where most tracks havealbum: None.Changes
parse_audio_playlist()now returnsNoneif not all tracks have album info, signaling to the caller to fall back to regular playlist parsingget_playlist()to check return valueOLAK5uy_mzYnlaHgFOvLaxqIPnnouEr-idiUn4NIM)Checklist
ruff,ruff format,mypy)