Rewrite of Spotify integration #30717
Conversation
5ba55b2 to
aa12871
Compare
|
Great progress! Hope we maintain the ability to have multiple spotify accounts in HA as the current implementation provides. I would guess the multiple instances of '.spotify_xxxx' files will no longer be needed. |
|
@Ahoui-group Yes, it supports multiple accounts. Cache files are no longer needed and not used. |
|
Wasn't 100% sure if it was in the code but will this also import existing configuration? Given it is updated to the new format? |
|
@dshokouhi No, it won't, you have to re-add/re-auth. |
|
good work @frenck ! wait for it! 👍 |
|
Sorry to ask but @frenck when do you think it would be released please? I m hoping it was already on the 0.104.2 of yesterday lol ... cant wait for it ;) |
|
@Electronlibre2012 This re-write introduces new features and will not be released with a bug fix release. I'm currently aiming for 0.105, however, there is no warranty it will make that release. |
|
thanks, ok...we have to wait... |
|
@frenck Great that you picked this up! |
|
OAuth2 via our account linking service is still the plan and it's possible with the OAuth2 helpers that Frenck has implemented in this PR. |
|
That is great to hear! |
00fdef6 to
e5440dd
Compare
|
ℹ️ Rebased PR onto latest |
| ) | ||
|
|
||
| # pylint: disable=protected-access | ||
| state = config_entry_oauth2_flow._encode_jwt(hass, {"flow_id": result["flow_id"]}) |
There was a problem hiding this comment.
Can we patch this encode function instead so we don't need to disable the pylint warning?
There was a problem hiding this comment.
I guess that will be unneeded hard since it has to be valid later on.
Followed the implementation by Paulus in Somfy for this.
IMHO, patching or using private methods are equally bad though 🤷♂
I added the pylint disable, because well, I think it is nice to do, I can simply remove it since we don't run it on tests.
There was a problem hiding this comment.
Ok. We should keep the disable so that editor linters and lazytox don't complain.
There was a problem hiding this comment.
In my opinion we should lint tests too.
There was a problem hiding this comment.
Totally agree! I lint my tests, but it has never been done @ HA. The amount of linter warnings is insane right now.
Breaking Change:
The Spotify platform has been rewritten to an integration. The configuration has been changed. Removing the current Spotify platform and setting up the integration from scratch is recommended.
The device aliases and the
spotify.play_playlistservice, have been removed. Be sure to remove calls to that service from your automations.If you use the Spotify platform at this moment:
configuration.yamlfile..spotify-token-cachefile from your configuration directory. It is no longer used, however, it does contain security credentials.Description:
I figured Spotify is probably something people use a lot. It works but could be better. A good excuse for me to play with OAuth2 in Home Assistant 😉
Changes:
spotify.play_playlist_service, usemedia_player.play_media(usingmedia_content_type=playlist) instead.Still to work on:
config_flow🥇__init__🥇media_player🥇Related issue (if applicable): fixes #27139
Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#11858
Example entry for
configuration.yaml(if applicable):Checklist:
tox. Your PR cannot be merged unless tests passIf user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
python3 -m script.hassfest.requirements_all.txtby runningpython3 -m script.gen_requirements_all..coveragerc.If the code does not interact with devices: