diff --git a/database/src/pool.rs b/database/src/pool.rs index 5269be392..c4f771263 100644 --- a/database/src/pool.rs +++ b/database/src/pool.rs @@ -219,14 +219,17 @@ pub trait Connection: Send + Sync { ) -> anyhow::Result<()>; /// Update a Try commit to have a `sha` and `parent_sha`. Will update the - /// status of the request too a ready state. + /// status of the request to the "artifacts_ready" state. + /// + /// Returns `true` if any benchmark request that was waiting for artifacts was found for the + /// given PR. async fn attach_shas_to_try_benchmark_request( &self, pr: u32, sha: &str, parent_sha: &str, commit_date: DateTime, - ) -> anyhow::Result<()>; + ) -> anyhow::Result; /// Add a benchmark job to the job queue and returns its ID, if it was not /// already in the DB previously. @@ -658,9 +661,10 @@ mod tests { let req = BenchmarkRequest::create_try_without_artifacts(42, "", ""); db.insert_benchmark_request(&req).await.unwrap(); - db.attach_shas_to_try_benchmark_request(42, "sha1", "sha-parent-1", Utc::now()) + assert!(db + .attach_shas_to_try_benchmark_request(42, "sha1", "sha-parent-1", Utc::now()) .await - .unwrap(); + .unwrap()); let req_db = db .load_pending_benchmark_requests() @@ -690,6 +694,21 @@ mod tests { .await; } + #[tokio::test] + async fn attach_shas_missing_try_request() { + run_postgres_test(|ctx| async { + let db = ctx.db(); + + assert!(!db + .attach_shas_to_try_benchmark_request(42, "sha1", "sha-parent-1", Utc::now()) + .await + .unwrap()); + + Ok(ctx) + }) + .await; + } + #[tokio::test] async fn enqueue_benchmark_job() { run_postgres_test(|ctx| async { diff --git a/database/src/pool/postgres.rs b/database/src/pool/postgres.rs index 7304123f4..be0554134 100644 --- a/database/src/pool/postgres.rs +++ b/database/src/pool/postgres.rs @@ -1717,21 +1717,21 @@ where sha: &str, parent_sha: &str, commit_date: DateTime, - ) -> anyhow::Result<()> { - self.conn() + ) -> anyhow::Result { + let modified_rows = self + .conn() .execute( - "UPDATE - benchmark_request - SET - tag = $1, - parent_sha = $2, - status = $3, - commit_date = $6 - WHERE - pr = $4 - AND commit_type = 'try' - AND tag IS NULL - AND status = $5;", + "UPDATE benchmark_request + SET + tag = $1, + parent_sha = $2, + status = $3, + commit_date = $6 + WHERE + pr = $4 + AND commit_type = 'try' + AND tag IS NULL + AND status = $5;", &[ &sha, &parent_sha, @@ -1744,7 +1744,7 @@ where .await .context("failed to attach SHAs to try benchmark request")?; - Ok(()) + Ok(modified_rows > 0) } async fn load_pending_benchmark_requests(&self) -> anyhow::Result { diff --git a/database/src/pool/sqlite.rs b/database/src/pool/sqlite.rs index b192ddd38..b4c318fce 100644 --- a/database/src/pool/sqlite.rs +++ b/database/src/pool/sqlite.rs @@ -1344,7 +1344,7 @@ impl Connection for SqliteConnection { _sha: &str, _parent_sha: &str, _commit_date: DateTime, - ) -> anyhow::Result<()> { + ) -> anyhow::Result { no_queue_implementation_abort!() } diff --git a/site/src/github.rs b/site/src/github.rs index a2a6e2dad..70c29f055 100644 --- a/site/src/github.rs +++ b/site/src/github.rs @@ -4,7 +4,6 @@ pub mod comparison_summary; use crate::api::github::Commit; use crate::job_queue::should_use_job_queue; use crate::load::{MissingReason, SiteCtxt, TryCommit}; -use chrono::{DateTime, Utc}; use serde::Deserialize; use std::sync::LazyLock; use std::time::Duration; @@ -234,25 +233,6 @@ pub async fn rollup_pr_number( .then_some(issue.number)) } -async fn attach_shas_to_try_benchmark_request( - conn: &dyn database::pool::Connection, - pr_number: u32, - commit: &TryCommit, - commit_date: DateTime, -) { - if let Err(e) = conn - .attach_shas_to_try_benchmark_request( - pr_number, - &commit.sha, - &commit.parent_sha, - commit_date, - ) - .await - { - log::error!("Failed to add shas to try commit: {e:?}"); - } -} - pub async fn enqueue_shas( ctxt: &SiteCtxt, gh_client: &client::Client, @@ -280,14 +260,14 @@ pub async fn enqueue_shas( let conn = ctxt.conn().await; let queued = if should_use_job_queue(pr_number) { - attach_shas_to_try_benchmark_request( - &*conn, + conn.attach_shas_to_try_benchmark_request( pr_number, - &try_commit, + &try_commit.sha, + &try_commit.parent_sha, commit_response.commit.committer.date, ) - .await; - true + .await + .map_err(|error| format!("Cannot attach SHAs to try benchmark request on PR {pr_number} and SHA {}: {error:?}", try_commit.sha))? } else { conn.pr_attach_commit( pr_number,