Read build-status and rustc-version directly from builds instead of release #2267
Conversation
github-actions
bot
added
the
S-waiting-on-review
| @@ -117,7 +117,7 @@ impl CrateDetails { | |||
| releases.readme, | |||
| releases.description_long, | |||
| releases.release_time, | |||
| releases.build_status, | |||
| COALESCE(builds.build_status, false) as build_status, | |||
There was a problem hiding this comment.
I think we should already either remove or just rename the now unused columns.
This would also surface places which we might have forgotten.
| @@ -136,7 +136,7 @@ impl CrateDetails { | |||
| releases.license, | |||
| releases.documentation_url, | |||
| releases.default_target, | |||
| releases.doc_rustc_version, | |||
| builds.rustc_version, | |||
There was a problem hiding this comment.
I think we should already either remove or just rename the now unused columns.
This would also surface places which we might have forgotten.
| LEFT JOIN LATERAL ( | ||
| SELECT * FROM builds | ||
| WHERE builds.rid = releases.id | ||
| ORDER BY builds.build_time |
There was a problem hiding this comment.
nit: the DESC should be on the same line as the ORDER BY for readablity
| LEFT JOIN LATERAL ( | ||
| SELECT build_status FROM builds | ||
| WHERE builds.rid = releases.id | ||
| ORDER BY builds.build_time |
There was a problem hiding this comment.
nit: the DESC should be on the same line as the ORDER BY for readablity
| LEFT JOIN LATERAL ( | ||
| SELECT * FROM builds | ||
| WHERE builds.rid = releases.id | ||
| ORDER BY builds.build_time |
There was a problem hiding this comment.
nit: the DESC should be on the same line as the ORDER BY for readablity
| @@ -90,7 +90,7 @@ pub(crate) fn get_releases( | |||
| INNER JOIN builds ON releases.id = builds.rid | |||
| LEFT JOIN repositories ON releases.repository_id = repositories.id | |||
| WHERE | |||
| ((NOT $3) OR (releases.build_status = FALSE AND releases.is_library = TRUE)) | |||
| ((NOT $3) OR (builds.build_status = FALSE AND releases.is_library = TRUE)) | |||
There was a problem hiding this comment.
perhaps I'm missing something, but this would change behaviour of the query, for the case when we have multiple builds for a release.
releases.build_status will contain the last build result.
So assuming that the last build was successful, and the first one broke, in the old query we wouldn't see that release in the result, because the last build succeeded. In the new query we would see the release in the result because an older build failed, while the most recent one passed.
| @@ -182,7 +188,10 @@ impl CrateDetails { | |||
| default_target: krate.get("default_target"), | |||
| doc_targets: MetaData::parse_doc_targets(krate.get("doc_targets")), | |||
| yanked: krate.get("yanked"), | |||
| rustdoc_css_file: get_correct_docsrs_style_file(krate.get("doc_rustc_version"))?, | |||
| rustdoc_css_file: krate | |||
There was a problem hiding this comment.
related question:
was there a time when we only stored records in the releases table? and nothing in builds?
Wouldn't this change not break old docs?
There was a problem hiding this comment.
If so that was before my time, would have to check the prod database dump
syphar
added
S-waiting-on-author
|
Was merged as part of #2422 |
This is a first step on preparing the database for having a release without any builds. I have a separate branch that deletes the columns that become unused here, but think that this should be deployed first so that it can be easily reverted if necessary.