search

mopidy / mopidy-soundcloud

Mopidy extension for playing music from SoundCloud
https://mopidy.com/ext/soundcloud/
MIT License
185 stars 59 forks source link

Fix/browsing #120

Closed Laurentww closed 3 years ago

Laurentww commented 3 years ago

Fixes lookup of wrong local uri in browsing the mopidy-soundcloud library

djmattyg007 commented 3 years ago

These changes appear to overlap with the changes you've made for #121.

Laurentww commented 3 years ago

Yes that's true, for some reason the subfolders don't show if these checks aren't here. It then shows this error: LookupError

These are also in #121 because without, browsing to songs would not be possible. If anyone has a different experience, it may be something weird in my setup.

djmattyg007 commented 3 years ago

If there's a common set of changes between PRs, you might want to convert one of them to a draft and make it clear that it will need to be rebased on top of the other PR once it's merged.

Laurentww commented 3 years ago

If the error isn't reproducible, I'll remove the PR.

kingosticks commented 3 years ago

What is your setup? How would I reproduce this?

Laurentww commented 3 years ago

After second try I see that not having the lookup uri-startswith-check doesn't prohibit lookup of folders and tracks. However, when browsing soundcloud folders, it does avoid extra erroneous api calls for tracks.

This PR tries to fix what does not necessarily needs to be fixed. It can be closed.

kingosticks commented 3 years ago

Was this specifically when using iris?

Laurentww commented 3 years ago

Yes it happens when browsing in Iris.

kingosticks commented 3 years ago

So that's the key to reproducing. Other web clients/frontends don't have the same odd browse+lookup pairing of calls. There's an issue about this, I think something was changed recently but I don't track iris very closely.

In summary, it is worth doing something sensible when clients make calls like this. Looking up directories is usually a client bug.

Laurentww commented 3 years ago

Then it might be an idea to leave it as is and update it on the next implementation of the lookup function. Thanks for following up!

kingosticks commented 3 years ago

We can always reopen. I think with your other fix this backend finally becomes usable again and we'll see more reports if this is a problem.