Skip to content

krate.search: Add support for ?ids[]=foo&ids[]=bar queries #3426

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

Merged
merged 1 commit into from
Mar 20, 2021

Conversation

Turbo87
Copy link
Member

@Turbo87 Turbo87 commented Mar 18, 2021

... to reduce unnecessary N+1 requests

This can be used in combination with e.g. #3405 to speed up the loading of crate descriptions.

@Turbo87 Turbo87 added C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works A-backend ⚙️ labels Mar 18, 2021
... to reduce unnecessary N+1 requests
@jtgeibel
Copy link
Member

We may want to put a limit on the number of crates. We limit pagination to 100 per page, but could consider a higher limit here. (This codebase has 60 direct dependencies, so 100+ is reasonable. Would it be easy to split >100 dependencies into multiple requests on the frontend?)

@Turbo87
Copy link
Member Author

Turbo87 commented Mar 20, 2021

Since this is using the same code paths as the regular search the default pagination options apply AFAICT.

splitting the request into multiple chunks on the frontend is not a problem.

@jtgeibel
Copy link
Member

Yeah, you're right. It works fine as-is.

@bors r+

@bors
Copy link
Contributor

bors commented Mar 20, 2021

📌 Commit 659c0e6 has been approved by jtgeibel

@bors
Copy link
Contributor

bors commented Mar 20, 2021

⌛ Testing commit 659c0e6 with merge 791d4d9...

@bors
Copy link
Contributor

bors commented Mar 20, 2021

☀️ Test successful - checks-actions
Approved by: jtgeibel
Pushing 791d4d9 to master...

@bors bors merged commit 791d4d9 into rust-lang:master Mar 20, 2021
@Turbo87 Turbo87 deleted the bulk-fetch branch March 21, 2021 09:36
@Turbo87
Copy link
Member Author

Turbo87 commented Jan 7, 2025

for future reference, the reason for this specific design was: https://github.com/emberjs/data/blob/v4.12.8/packages/adapter/src/rest.ts#L709

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-backend ⚙️ C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants