Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Backend.lookup should take a list of URIs #2116

Closed
kingosticks opened this issue Aug 8, 2023 · 1 comment · Fixed by #2145
Closed

Backend.lookup should take a list of URIs #2116

kingosticks opened this issue Aug 8, 2023 · 1 comment · Fixed by #2145
Assignees
@rchatrath7
Labels
A-core Area: Core layer C-enhancement Category: A PR with an enhancement or an issue with an enhancement proposal
Milestone
v4.0.0

Comments

@kingosticks
Copy link
Member

kingosticks commented Aug 8, 2023 •
edited
Loading

Currently Backend.lookup() is called by Core.LibraryController.lookup() for each URI in turn. This prevents backends from potentially batching up the work. In some cases this wastes valuable API quota resources. We should make lookup() work like get_images() which already supports lists.

This will be a change to the Backend API so care will be needed to avoid breaking existing backends.
@kingosticks kingosticks added C-enhancement Category: A PR with an enhancement or an issue with an enhancement proposal A-core Area: Core layer labels Aug 8, 2023
@kingosticks kingosticks added this to the Next breaking release milestone Oct 17, 2023
@jodal jodal modified the milestones: Next breaking release, v4.0.0 Oct 29, 2023
@rchatrath7
Copy link
Contributor

rchatrath7 commented Jan 1, 2024 •
edited
Loading

Hi @kingosticks @jodal would I be able to be assigned to this? I think I am close to a PR for this.

rchatrath7 pushed a commit to rchatrath7/mopidy that referenced this issue Jan 1, 2024
mpd: Modify Backend.lookup to accept a list of URIs
b3f5a85 
Resolves mopidy#2116.

Changes `uri` param from `Uri` in `Backend.lookup` to `list[Uri]`. Also
enforces the return type on `Backend.lookup` to be `Dict`. Changes
`CoreLibrary.lookup` to pass in list of URIs into `Backend.lookup`
instead of calling it individually.
@rchatrath7 rchatrath7 mentioned this issue Jan 1, 2024
Merged
rchatrath7 pushed a commit to rchatrath7/mopidy that referenced this issue Jan 1, 2024
mpd: Modify Backend.lookup to accept a list of URIs
4187b13 
Resolves mopidy#2116.

Changes `uri` param from `Uri` in `Backend.lookup` to `list[Uri]`. Also
enforces the return type on `Backend.lookup` to be `dict`. Changes
`core.Library.lookup` to pass in list of URIs into `Backend.lookup`
instead of calling it individually.
rchatrath7 pushed a commit to rchatrath7/mopidy that referenced this issue Jan 1, 2024
mpd: Modify Backend.lookup to accept a list of URIs
84cb53e 
Resolves mopidy#2116.

Changes `uri` param from `Uri` in `Backend.lookup` to `list[Uri]`. Also
enforces the return type on `Backend.lookup` to be `dict`. Changes
`core.Library.lookup` to pass in list of URIs into `Backend.lookup`
instead of calling it individually.
rchatrath7 pushed a commit to rchatrath7/mopidy that referenced this issue Jan 1, 2024
mpd: Modify Backend.lookup to accept a list of URIs
f56f336 
Resolves mopidy#2116.

Changes `uri` param from `Uri` in `Backend.lookup` to `list[Uri]`. Also
enforces the return type on `Backend.lookup` to be `dict`. Changes
`core.Library.lookup` to pass in list of URIs into `Backend.lookup`
instead of calling it individually.

Update test to batch uris for multiple backends
rchatrath7 pushed a commit to rchatrath7/mopidy that referenced this issue Mar 2, 2024
@rchatrath7
mpd: Modify Backend.lookup to accept a list of URIs
2765861 
Resolves mopidy#2116.

Changes `uri` param from `Uri` in `Backend.lookup` to `list[Uri]`. Also
enforces the return type on `Backend.lookup` to be `dict`. Changes
`core.Library.lookup` to pass in list of URIs into `Backend.lookup`
instead of calling it individually.

Update test to batch uris for multiple backends
rchatrath7 pushed a commit to rchatrath7/mopidy that referenced this issue Mar 5, 2024
@rchatrath7
mpd: Add Backend.lookup_many as replacement to Backend.lookup to acce…
026034e 
…pt list of URIs

Resolves mopidy#2116.

Adds new method `Backend.lookup_many` as a replacement to
`Backend.lookup` which accepts multiple URIs for lookup instead of a
single URI. Provides a default implementation for `Backend.lookup_many`
that iteratively calls `Backend.lookup` for backwards compatibility with
existing backends.

Update test to batch uris for multiple backends
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rchatrath7 rchatrath7

Labels
A-core Area: Core layer C-enhancement Category: A PR with an enhancement or an issue with an enhancement proposal
Projects
None yet
Milestone
v4.0.0
Development

Successfully merging a pull request may close this issue.

Modify Backend.lookup to accept list of URIs rchatrath7/mopidy
3 participants
@jodal @kingosticks @rchatrath7