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

show in-progress builds in build queue & build-list pages #2533

Merged
merged 8 commits into from
Sep 22, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions migrations/20240624085737_build-status-idx.down.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
DROP INDEX builds_build_status_idx;
1 change: 1 addition & 0 deletions migrations/20240624085737_build-status-idx.up.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
CREATE INDEX builds_build_status_idx ON builds USING btree (build_status ASC);
46 changes: 22 additions & 24 deletions src/web/builds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,21 +107,24 @@ pub(crate) async fn build_list_json_handler(
get_builds(&mut conn, &name, &version)
.await?
.iter()
.map(|build| {
.filter_map(|build| {
if build.build_status == BuildStatus::InProgress {
return None;
}
// for backwards compatibility in this API, we
// * convert the build status to a boolean
// * already filter out in-progress builds
//
// even when we start showing in-progress builds in the UI,
// we might still not show them here for backwards
// compatibility.
serde_json::json!({
Some(serde_json::json!({
"id": build.id,
"rustc_version": build.rustc_version,
"docsrs_version": build.docsrs_version,
"build_status": build.build_status.is_success(),
"build_time": build.build_time,
})
}))
})
.collect::<Vec<_>>(),
),
Expand Down Expand Up @@ -245,8 +248,7 @@ async fn get_builds(
INNER JOIN crates ON releases.crate_id = crates.id
WHERE
crates.name = $1 AND
releases.version = $2 AND
builds.build_status != 'in_progress'
releases.version = $2
ORDER BY id DESC"#,
name,
version.to_string(),
Expand Down Expand Up @@ -548,29 +550,25 @@ mod tests {

let response = env.frontend().get("/crate/foo/0.1.0/builds").send()?;

// FIXME: temporarily we don't show in-progress releases anywhere, which means we don't
// show releases without builds anywhere.
assert_eq!(response.status(), StatusCode::NOT_FOUND);

// assert_cache_control(&response, CachePolicy::NoCaching, &env.config());
// let page = kuchikiki::parse_html().one(response.text()?);
assert_cache_control(&response, CachePolicy::NoCaching, &env.config());
let page = kuchikiki::parse_html().one(response.text()?);

// let rows: Vec<_> = page
// .select("ul > li a.release")
// .unwrap()
// .map(|row| row.text_contents())
// .collect();
let rows: Vec<_> = page
.select("ul > li a.release")
.unwrap()
.map(|row| row.text_contents())
.collect();

// assert!(rows.is_empty());
assert!(rows.is_empty());

// let warning = page
// .select_first(".warning")
// .expect("missing warning element")
// .text_contents();
let warning = page
.select_first(".warning")
.expect("missing warning element")
.text_contents();

// assert!(warning.contains("has not built"));
// assert!(warning.contains("queued"));
// assert!(warning.contains("open an issue"));
assert!(warning.contains("has not built"));
assert!(warning.contains("queued"));
assert!(warning.contains("open an issue"));

Ok(())
});
Expand Down
38 changes: 17 additions & 21 deletions src/web/crate_details.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,8 +374,7 @@ pub(crate) async fn releases_for_crate(
FROM releases
INNER JOIN release_build_status ON releases.id = release_build_status.rid
WHERE
releases.crate_id = $1 AND
release_build_status.build_status != 'in_progress'"#,
releases.crate_id = $1"#,
crate_id,
)
.fetch(&mut *conn)
Expand Down Expand Up @@ -1341,25 +1340,22 @@ mod tests {
.create()?;

let response = env.frontend().get("/crate/foo/latest").send()?;
// FIXME: temporarily we don't show in-progress releases anywhere, which means we don't
// show releases without builds anywhere.
assert_eq!(response.status(), StatusCode::NOT_FOUND);

// let page = kuchikiki::parse_html().one(response.text()?);
// let link = page
// .select_first("a.pure-menu-link[href='/crate/foo/0.1.0']")
// .unwrap();

// assert_eq!(
// link.as_node()
// .as_element()
// .unwrap()
// .attributes
// .borrow()
// .get("title")
// .unwrap(),
// "foo-0.1.0 is currently being built"
// );

let page = kuchikiki::parse_html().one(response.text()?);
let link = page
.select_first("a.pure-menu-link[href='/crate/foo/0.1.0']")
.unwrap();

assert_eq!(
link.as_node()
.as_element()
.unwrap()
.attributes
.borrow()
.get("title")
.unwrap(),
"foo-0.1.0 is currently being built"
);

Ok(())
});
Expand Down
91 changes: 67 additions & 24 deletions src/web/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,27 @@ impl MatchedRelease {
}
}

fn semver_match<'a, F: Fn(&Release) -> bool>(
releases: &'a [Release],
req: &VersionReq,
filter: F,
) -> Option<&'a Release> {
if let Some(release) = releases
.iter()
.filter(|release| filter(release))
.find(|release| req.matches(&release.version))
{
Some(release)
} else if req == &VersionReq::STAR {
// semver `*` does not match pre-releases.
// When someone wants the latest release and we have only pre-releases
// just return the latest prerelease.
return releases.iter().find(|release| filter(release));
} else {
None
}
syphar marked this conversation as resolved.
Show resolved Hide resolved
}

/// Checks the database for crate releases that match the given name and version.
///
/// `version` may be an exact version number or loose semver version requirement. The return value
Expand Down Expand Up @@ -307,14 +328,12 @@ async fn match_version(
ReqVersion::Semver(version_req) => version_req.clone(),
};

// when matching semver requirements, we only want to look at non-yanked releases.
let flt = |r: &&Release| r.yanked == Some(false);

if let Some(release) = releases
.iter()
.filter(flt)
.find(|release| req_semver.matches(&release.version))
{
// when matching semver requirements,
// we generally only want to look at non-yanked releases,
// excluding releases which just contain in-progress builds
if let Some(release) = semver_match(&releases, &req_semver, |r: &Release| {
r.build_status != BuildStatus::InProgress && (r.yanked.is_none() || r.yanked == Some(false))
}) {
return Ok(MatchedRelease {
name: name.to_owned(),
corrected_name,
Expand All @@ -324,22 +343,17 @@ async fn match_version(
});
}

// semver `*` does not match pre-releases.
// When someone wants the latest release and we have only pre-releases
// just return the latest prerelease.
if req_semver == VersionReq::STAR {
return releases
.iter()
.find(flt)
.cloned()
.map(|release| MatchedRelease {
name: name.to_owned(),
corrected_name: corrected_name.clone(),
req_version: input_version.clone(),
release,
all_releases: releases,
})
.ok_or(AxumNope::VersionNotFound);
// when we don't find any match with "normal" releases, we also look into in-progress releases
if let Some(release) = semver_match(&releases, &req_semver, |r: &Release| {
r.yanked.is_none() || r.yanked == Some(false)
}) {
return Ok(MatchedRelease {
name: name.to_owned(),
corrected_name,
req_version: input_version.clone(),
release: release.clone(),
all_releases: releases,
});
}

// Since we return with a CrateNotFound earlier if the db reply is empty,
Expand Down Expand Up @@ -1050,6 +1064,35 @@ mod test {
});
}

#[test]
fn in_progress_releases_are_ignored_when_others_match() {
async_wrapper(|env| async move {
let db = env.async_db().await;

// normal release
release("1.0.0", &env).await;

// in progress release
env.async_fake_release()
.await
.name("foo")
.version("1.1.0")
.builds(vec![
FakeBuild::default().build_status(BuildStatus::InProgress)
])
.create_async()
.await?;

// STAR gives me the prod release
assert_eq!(version(Some("*"), db).await, exact("1.0.0"));

// exact-match query gives me the in progress release
assert_eq!(version(Some("=1.1.0"), db).await, exact("1.1.0"));

Ok(())
})
}

#[test]
// https://github.com/rust-lang/docs.rs/issues/1682
fn prereleases_are_considered_when_others_dont_match() {
Expand Down
3 changes: 2 additions & 1 deletion src/web/page/templates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,8 @@ impl IconType {
IconType::Brand => font_awesome_as_a_crate::Type::Brands,
};

let icon_file_string = font_awesome_as_a_crate::svg(type_, icon_name).unwrap_or("");
let icon_file_string = font_awesome_as_a_crate::svg(type_, icon_name)
.map_err(|err| rinja::Error::Custom(Box::new(err)))?;

let mut classes = vec!["fa-svg"];
if fw {
Expand Down
28 changes: 24 additions & 4 deletions src/web/releases.rs
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,8 @@ pub(crate) async fn activity_handler(mut conn: DbConnection) -> AxumResult<impl
struct BuildQueuePage {
description: &'static str,
queue: Vec<QueuedCrate>,
active_deployments: Vec<String>,
active_cdn_deployments: Vec<String>,
in_progress_builds: Vec<(String, String)>,
csp_nonce: String,
}

Expand All @@ -793,8 +794,9 @@ impl_axum_webpage! { BuildQueuePage }
pub(crate) async fn build_queue_handler(
Extension(build_queue): Extension<Arc<BuildQueue>>,
Extension(pool): Extension<Pool>,
mut conn: DbConnection,
) -> AxumResult<impl IntoResponse> {
let (queue, active_deployments) = spawn_blocking(move || {
let (queue, active_cdn_deployments) = spawn_blocking(move || {
let mut queue = build_queue.queued_crates()?;
for krate in queue.iter_mut() {
// The priority here is inverted: in the database if a crate has a higher priority it
Expand All @@ -820,10 +822,28 @@ pub(crate) async fn build_queue_handler(
})
.await?;

let in_progress_builds: Vec<(String, String)> = sqlx::query!(
r#"SELECT
crates.name,
releases.version
FROM builds
INNER JOIN releases ON releases.id = builds.rid
INNER JOIN crates ON releases.crate_id = crates.id
WHERE
builds.build_status = 'in_progress'
ORDER BY builds.id ASC"#
)
.fetch_all(&mut *conn)
.await?
.into_iter()
.map(|rec| (rec.name, rec.version))
.collect();

Ok(BuildQueuePage {
description: "crate documentation scheduled to build & deploy",
queue,
active_deployments,
active_cdn_deployments,
in_progress_builds,
csp_nonce: String::new(),
})
}
Expand Down Expand Up @@ -1695,7 +1715,7 @@ mod tests {

let empty = kuchikiki::parse_html().one(web.get("/releases/queue").send()?.text()?);
assert!(empty
.select(".release > strong")
.select(".release > div > strong")
.expect("missing heading")
.any(|el| el.text_contents().contains("active CDN deployments")));

Expand Down
Loading