New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Guard browsing Spotify if setup failed #65074
Guard browsing Spotify if setup failed #65074
Conversation
Hey there @frenck, mind taking a look at this pull request as it has been labeled with an integration ( |
info = list(hass.data[DOMAIN].values())[0] | ||
info = next(iter(hass.data[DOMAIN].values()), None) | ||
if not info: | ||
raise BrowseError("No Spotify accounts available") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we catch this inside Sonos?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this makes a lot of sense to do here, mainly because anything else can just use it without having to worry about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I meant catch BrowseError
in Sonos. The check here is indeed correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that's necessary. It should only hit this codepath if the browser is explicitly trying to browse into Spotify. It'll only hit this at the root level is Spotify is the only source available.
Co-authored-by: Franck Nijhof <frenck@frenck.nl>
Co-authored-by: Franck Nijhof <frenck@frenck.nl>
Proposed change
If Spotify is configured but hasn't successfully loaded it can lead to errors when browsing from another integration (e.g., Sonos).
Type of change
Additional information
Checklist
black --fast homeassistant tests
)The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: