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

Add description, homepage, documentation and repository fields for versions #9998

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

Conversation

Turbo87
Copy link
Member

@Turbo87 Turbo87 commented Nov 19, 2024

We currently save these fields only per crate, not per version. This means that for crates publishing bug fixes to older major versions, these fields might get overwritten with outdated metadata.

The solution to this is to read these per crate metadata fields from the "default version" of a crate instead.

This PR introduces four more metadata columns to the versions table (description, homepage, documentation and repository) and exposes their values on the API. The rest of the code is not using these values yet since they will need to be backfilled from the stored crate files first. Once that has happened we could remove the fields from the crates table and use the content from the default version of a crate instead.

Potential downsides of this:

  • database size: since we are storing mostly similar data for each version this might grow the database size considerably.
  • database dump size: same as above, though compression should hopefully get rid of most of the overhead.
  • database dump breaking change: if we remove the existing columns of the crates table, or if we stop updating them, this will essentially be a breaking change to the database dump schema. while we should not see this as a hard blocker, we should be careful to not introduce breaking changes too often.
  • more joins in queries needed: this might have a performance impact, but with the existing indexes I expect this to be manageable, and we're already pulling in the default version for the relevant endpoint anyway, so the additional overhead might actually be zero.

Related:

@Turbo87 Turbo87 added C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear A-backend ⚙️ labels Nov 19, 2024
@Turbo87 Turbo87 requested a review from a team November 19, 2024 09:46
@Turbo87
Copy link
Member Author

Turbo87 commented Nov 19, 2024

RE database dump breaking change: I guess we could also denormalize the data from the versions to the crates table whenever we update the default version 🤔

Copy link

codecov bot commented Nov 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.23%. Comparing base (612dd59) to head (2d07f82).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9998      +/-   ##
==========================================
+ Coverage   89.22%   89.23%   +0.01%     
==========================================
  Files         295      295              
  Lines       30839    30863      +24     
==========================================
+ Hits        27516    27541      +25     
+ Misses       3323     3322       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-backend ⚙️ C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear
Projects
Status: For next meeting
Development

Successfully merging this pull request may close these issues.

1 participant