Improvement of MusicMe music provider for API signin, and fix the parsing of the "streamable" fields and playlists ids#4029
Improvement of MusicMe music provider for API signin, and fix the parsing of the "streamable" fields and playlists ids#4029mintgrey wants to merge 15 commits into
Conversation
|
@JulienDeveaux could you please have a look at this? |
There was a problem hiding this comment.
Pull request overview
Improves the existing MusicMe music provider by adding an API-based signin alternative to the existing scraped-web login, broadening the "streamable" filter from == 2 to != 0 so albums/tracks flagged with other non-zero values are not hidden, and stripping the pl- prefix from playlist IDs so the dataservice playlist endpoints work for those entries.
Changes:
- Adds a
SIGNIN_BY_APIboolean config entry and a new_signin_by_apimethod that authenticates by hitting/medialibrary/signinvia_api_get. - Replaces all
streamable == 2checks withstreamable != 0in search, browse, recommendations, top tracks, radio track selection, and parsers. - Normalizes playlist IDs in
_parse_playlistby stripping apl-prefix; updates one streaming test fixture to match the new streamable semantics.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| music_assistant/providers/musicme/constants.py | Adds the SIGNIN_BY_API config key constant. |
| music_assistant/providers/musicme/init.py | Registers the new boolean config entry for API signin. |
| music_assistant/providers/musicme/provider.py | Branches init between web login and new _signin_by_api; loosens streamable checks; strips pl- from playlist ids. |
| tests/providers/musicme/test_streaming.py | Updates a fixture from streamable: 1 to streamable: 0 to match new semantics. |
Comments suppressed due to low confidence (1)
music_assistant/providers/musicme/provider.py:60
- [PROBLEM] The import group is no longer alphabetically sorted (
SIGNIN_BY_APIshould come afterPARTNER_ID). Ruff/isort in pre-commit will flag this.
from .constants import (
SIGNIN_BY_API,
CLIENT_JSON,
DATASERVICE_BASE,
LOGIN_URL,
PARTNER_ID,
STREAM_BASE,
VALID_ID_RE,
WEB_BASE,
| response = await self._api_get( | ||
| f"/medialibrary/signin" | ||
| f"?channel=65777&lang=fr&format=json&client=%7B%22type%22%3A%22desktop-web%22%2C%22context%22%3A%22pro.bib.musicme.com%22%7D" | ||
| f"&key=sKTBA7ybW3nvCUQ6&nocrypt=0&login={login}&password={password}" | ||
| ) |
| def _parse_playlist(self, playlist_obj: dict[str, Any]) -> Playlist: | ||
| """Parse a MusicMe playlist object to a Music Assistant Playlist.""" | ||
| playlist_id = str(playlist_obj.get("id", "")) | ||
| playlist_id = (str(playlist_obj.get("id", ""))).removeprefix("pl-") |
|
Hi, I'll take a look next week :) Linking the discussion : https://github.com/orgs/music-assistant/discussions/5421 |
|
@mintgrey As you address each copilot comment then please add a response |
|
Hi @mintgrey, I did some tests, and it turns out my account on the main domain also work in the SIGNIN_BY_API method.
I think we can make the previous auth method as a fallback if the SIGNIN_BY_API fails to auth or even remove it. Don't forget to edit / create new tests for the new auth method. Thank for your work on this 👍 |
Hi @JulienDeveaux, Thanks a lot, and good news if it's work more widely than on my only account :) ! Indeed, I've begin an need to continue ASAP to re(re, re)work the PR to properly include each advice from Copilot and to edit the test plan. For the checkbox, we can maybe put it after the login/password fiels, and explicitely label it as "Web login as fallback method" to keep the previous method "in case of" ? Stay tuned :) |
…dentation corrections
|
Hi @mintgrey, |
|
OK, I understand. And just for be sure (and for the beauty of the exercise ;), maybe we can put it only in "advanced" mode ? |
|
That's a good compromise I think |
|
👍 I will take a look at this ! |
|
I am just going to mark this one as draft so we can keep track of those that need our attention. Please mark as ready for review when you want us to look again. |
What does this implement/fix?
This PR adds/corrects 3 functionalities as discussed in Discussion #5421 :
The modifications are tested locally for several days on my HA/MA installation, and it works like a charm !
Types of changes
bugfixnew-featureenhancementnew-providerbreaking-changerefactordocumentationmaintenancecidependenciesChecklist
pre-commit run --all-filespasses.pytestpasses, and tests have been added/updated undertests/where applicable.music-assistant/modelsis linked.music-assistant/frontendis linked.P.S. This is my first PR on github, I'm listening for any advice or correction !