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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Looking up tracks with find_exact is very slow #1008

Closed
dirkgroenen opened this Issue Feb 26, 2015 · 12 comments

Comments

5 participants
@dirkgroenen
Contributor

dirkgroenen commented Feb 26, 2015

For my Mopify web client I have to retrieve track object for a list of URI's. To accomplish this I use the find_exact method with a list of uris. However, a request containing 150 uris takes about 11 seconds before it returns a result.

Is there an other way to load those 'non-mopidy' tracks into the tracklist faster? More details about this issue can be found in #62.

@jodal

This comment has been minimized.

Member

jodal commented Feb 26, 2015

In our draft for the Core API v1.0, we've changed library.lookup() to take a list of URIs. With time, that should solve your use case.

@kingosticks

This comment has been minimized.

Member

kingosticks commented Feb 26, 2015

Would it make sense (if not already planned) for the tracklist's add method
to also take a list of uris?
On 26 Feb 2015 12:44, "Stein Magnus Jodal" notifications@github.com wrote:

In our draft for the Core API v1.0, we've changed library.lookup() to
take a list of URIs. With time, that should solve your use case.


Reply to this email directly or view it on GitHub
#1008 (comment).

@dirkgroenen

This comment has been minimized.

Contributor

dirkgroenen commented Feb 26, 2015

Any indication on when V1.0 is going to be released?

@jodal

This comment has been minimized.

Member

jodal commented Feb 26, 2015

No, we don't have that. We've started to include some of the backwards compatible parts of the Core API v1.0 in the upcoming v0.20, but changing the signature of lookup() is quite backwards incompatible, so that will probably be left for v1.0 proper.

@jodal

This comment has been minimized.

Member

jodal commented Feb 26, 2015

Yes, I think it makes very much sense to extend tracklist.add() with an uris keyword argument, and remove the current uri argument in v1.0.

@jodal

This comment has been minimized.

Member

jodal commented Feb 26, 2015

That said, a revamp of the tracklist as a whole is deferred to v2.0, mostly to draw a line and scope down the amount of changes in v1.0 and work required to get it out.

@adamcik

This comment has been minimized.

Member

adamcik commented Mar 14, 2015

Shouldn't we be able to do a lookup(uris) that does an if isinstance(uris, basestring): uris = [uris] to get this moving earlier?

@jodal

This comment has been minimized.

Member

jodal commented Mar 14, 2015

Assuming nobody uses lookup() with uri as a keyword argument, we should be able to change it to uris and add a string check for backwards compat.

@jodal jodal modified the milestones: v0.20 - Audio cleanup 1, v0.21 - Gapless playback Mar 14, 2015

@adamcik

This comment has been minimized.

Member

adamcik commented Mar 14, 2015

Or just lookup(uri=None, uris=None) and tracklist.add(uri=None, uris=None) and mark the uri case as deprecated and to be removed in 1.0 I guess.

@dirkgroenen

This comment has been minimized.

Contributor

dirkgroenen commented Mar 15, 2015

Must say that adding a uris parameter to the lookup method would save a big part of the problems I'm currently en counting when adding 'external' tracks. 👍

@tkem

This comment has been minimized.

Member

tkem commented Mar 15, 2015

Assuming nobody uses lookup() with uri as a keyword argument...

Since Mopidy.js introduced "by-position-or-by-name" calling convention, I know of at least one client that calls lookup() with uri as a keyword argument.

@dirkgroenen

This comment has been minimized.

Contributor

dirkgroenen commented Mar 15, 2015

Mopify also uses lookup() with uri as keyword when opening playlists.

adamcik added a commit to adamcik/mopidy that referenced this issue Mar 17, 2015

core: Add uris argument to library.lookup (Fixes mopidy#1008)
For now this doesn't add any corresponding APIs to backends, or for that matter
tracklist.add(uris). This is just to get the API in for clients in 0.20.

@jodal jodal modified the milestones: v0.20 - Audio cleanup 1, v0.21 - Gapless playback Mar 17, 2015

@jodal jodal added 1 - Ready and removed 1 - Ready labels Mar 18, 2015

@adamcik adamcik closed this in fdc84c3 Mar 18, 2015

jodal added a commit that referenced this issue Mar 18, 2015

Merge pull request #1047 from adamcik/fix/1008-add-uris-to-lookup
core: Add uris argument to library.lookup (Fixes #1008)

@jodal jodal removed the 2 - Working label Mar 18, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment