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.

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

controllers/version: Add traits to help reduce data loading #10603

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

eth3lbert
Copy link
Contributor

@eth3lbert eth3lbert commented Feb 17, 2025

This PR primarily addresses three things:

  • Loading Crate and Version in a single query.
  • Adding helper traits to allow customization of the required columns to load and validation.
  • Avoiding loading the entire Crate when only Version is needed.

First, I assume that in most cases, the given inputs are valid, then it would be great to load them in a single query.

Second, since most endpoints do not require all columns from Crate and Version, it would be nice to load only the necessary columns to reduce data loading.

Third, when only Version is required, it would be ideal to load only the ID of Crate to determine its existence.

I hope this doesn't overly complicate things :)

@eth3lbert eth3lbert added C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear A-backend ⚙️ labels Feb 17, 2025
@eth3lbert eth3lbert requested a review from Turbo87 February 17, 2025 13:50
Instead of loading them individually, this gets both in one query.
I think that in most cases, the request to query for `Crate` and
`Version` will have valid input, so this seems like a better approach,
even though it might only give a small performance boost.
…`crate_and_version()` fn

Since most endpoints don't need all of the columns for `crates` and
`versions`, it would be great to only load the columns we actually need
to make things faster.
This will enable other endpoints to use the function to make a base
query and customize the needed columns.
@eth3lbert
Copy link
Contributor Author

Rebased and conflict resolved!

Copy link
Member

@Turbo87 Turbo87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm somewhat unsure whether the extra complexity is worth it. @LawnGnome any opinions?

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 C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants