-
Notifications
You must be signed in to change notification settings - Fork 211
change latest version ID on crate to match CrateDetails.latest_version #1546
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
change latest version ID on crate to match CrateDetails.latest_version #1546
Conversation
3e7ec5b
to
ef71b8a
Compare
ef71b8a
to
c2d4296
Compare
@Nemo157 would you mind helping review this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, tested locally and appears to behave correctly.
I assume for the migration we just need to stop building, {update server, run migration} (in parallel/either order), then start building again?
yep, that would be one way. Another way would be so move the data migration after the deploy/restart. Since we're only updating data and no schema, this would be fine. The migration is not really optimized, so it will take some time (locally it were 10 minutes) I'll fix the conflicts with master, then we can merge. |
conn, | ||
&metadata_pkg.name, | ||
&metadata_pkg.version, | ||
&metadata_pkg.version, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that's fine, it only matters for the frontend templates.
conn, | ||
&metadata_pkg.name, | ||
&metadata_pkg.version, | ||
&metadata_pkg.version, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that's fine, it only matters for the frontend templates.
This seems to be tested and approved now, I'll merge it when I have time to also deploy it, so this migration is not blocking anyone. |
in all release-lists the one version we show should be the latest version (non-yanked and non-prerelease) to match the logic in the rustdoc page header. Otherwise we are showing releases in the lists that directly lead to a page with a go to latest version link.
Now
crates.latest_version_id
is calculated with the same logic and updated after each build.This is a preparation for #708, which I would do when #1521 is merged. Then it's just switching the calculated subquery to just using
crates.latest_version_id
.The data migration is a very simple impl, and takes 15 minutes on my machine (with macos virtualized docker). IMHO it's fine running that long on prod, of course I can improve speed when needed.