diff --git a/database/src/lib.rs b/database/src/lib.rs index fe9a43810..1fa9b2350 100644 --- a/database/src/lib.rs +++ b/database/src/lib.rs @@ -805,17 +805,23 @@ pub enum BenchmarkRequestStatus { Completed, } -impl fmt::Display for BenchmarkRequestStatus { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { +impl BenchmarkRequestStatus { + pub fn as_str(&self) -> &str { match self { - BenchmarkRequestStatus::WaitingForArtifacts => write!(f, "waiting_for_artifacts"), - BenchmarkRequestStatus::ArtifactsReady => write!(f, "artifacts_ready"), - BenchmarkRequestStatus::InProgress => write!(f, "in_progress"), - BenchmarkRequestStatus::Completed => write!(f, "completed"), + BenchmarkRequestStatus::WaitingForArtifacts => "waiting_for_artifacts", + BenchmarkRequestStatus::ArtifactsReady => "artifacts_ready", + BenchmarkRequestStatus::InProgress => "in_progress", + BenchmarkRequestStatus::Completed => "completed", } } } +impl fmt::Display for BenchmarkRequestStatus { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.as_str()) + } +} + impl<'a> tokio_postgres::types::FromSql<'a> for BenchmarkRequestStatus { fn from_sql( ty: &tokio_postgres::types::Type, @@ -825,10 +831,10 @@ impl<'a> tokio_postgres::types::FromSql<'a> for BenchmarkRequestStatus { let s: &str = <&str as tokio_postgres::types::FromSql>::from_sql(ty, raw)?; match s { - "waiting_for_artifacts" => Ok(Self::WaitingForArtifacts), - "artifacts_ready" => Ok(Self::ArtifactsReady), - "in_progress" => Ok(Self::InProgress), - "completed" => Ok(Self::Completed), + x if x == Self::WaitingForArtifacts.as_str() => Ok(Self::WaitingForArtifacts), + x if x == Self::ArtifactsReady.as_str() => Ok(Self::ArtifactsReady), + x if x == Self::InProgress.as_str() => Ok(Self::InProgress), + x if x == Self::Completed.as_str() => Ok(Self::Completed), other => Err(format!("unknown benchmark_request_status '{other}'").into()), } } @@ -856,30 +862,12 @@ pub enum BenchmarkRequestType { Release { tag: String }, } -impl BenchmarkRequestType { - pub fn commit_type_str(&self) -> &str { - match self { - BenchmarkRequestType::Try { - sha: _, - parent_sha: _, - pr: _, - } => "try", - BenchmarkRequestType::Master { - sha: _, - parent_sha: _, - pr: _, - } => "master", - BenchmarkRequestType::Release { tag: _ } => "release", - } - } -} - impl fmt::Display for BenchmarkRequestType { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { BenchmarkRequestType::Try { .. } => write!(f, "try"), BenchmarkRequestType::Master { .. } => write!(f, "master"), - BenchmarkRequestType::Release { tag: _ } => write!(f, "release"), + BenchmarkRequestType::Release { .. } => write!(f, "release"), } } } @@ -975,19 +963,15 @@ impl BenchmarkRequest { BenchmarkRequestType::Try { pr, .. } | BenchmarkRequestType::Master { pr, .. } => { Some(pr) } - BenchmarkRequestType::Release { tag: _ } => None, + BenchmarkRequestType::Release { .. } => None, } } - pub fn commit_type(&self) -> &str { - self.commit_type.commit_type_str() - } - pub fn parent_sha(&self) -> Option<&str> { match &self.commit_type { BenchmarkRequestType::Try { parent_sha, .. } => parent_sha.as_deref(), BenchmarkRequestType::Master { parent_sha, .. } => Some(parent_sha), - BenchmarkRequestType::Release { tag: _ } => None, + BenchmarkRequestType::Release { .. } => None, } } } diff --git a/database/src/pool.rs b/database/src/pool.rs index 5597a8ba2..458f08af7 100644 --- a/database/src/pool.rs +++ b/database/src/pool.rs @@ -181,8 +181,11 @@ pub trait Connection: Send + Sync { async fn purge_artifact(&self, aid: &ArtifactId); /// Add an item to the `benchmark_requests`, if the `benchmark_request` - /// exists it will be ignored - async fn insert_benchmark_request(&self, benchmark_request: &BenchmarkRequest); + /// exists an Error will be returned + async fn insert_benchmark_request( + &self, + benchmark_request: &BenchmarkRequest, + ) -> anyhow::Result<()>; /// Gets the benchmark requests matching the status. Optionally provide the /// number of days from whence to search from @@ -436,12 +439,20 @@ mod tests { ); let db = db.connection().await; - db.insert_benchmark_request(&master_benchmark_request).await; - db.insert_benchmark_request(&try_benchmark_request).await; + db.insert_benchmark_request(&master_benchmark_request) + .await + .unwrap(); + db.insert_benchmark_request(&try_benchmark_request) + .await + .unwrap(); db.insert_benchmark_request(&release_benchmark_request) - .await; + .await + .unwrap(); // duplicate insert - db.insert_benchmark_request(&master_benchmark_request).await; + assert!(db + .insert_benchmark_request(&master_benchmark_request) + .await + .is_err()); Ok(ctx) }) @@ -484,10 +495,15 @@ mod tests { ); let db = db.connection().await; - db.insert_benchmark_request(&master_benchmark_request).await; - db.insert_benchmark_request(&try_benchmark_request).await; + db.insert_benchmark_request(&master_benchmark_request) + .await + .unwrap(); + db.insert_benchmark_request(&try_benchmark_request) + .await + .unwrap(); db.insert_benchmark_request(&release_benchmark_request) - .await; + .await + .unwrap(); let requests = db .get_benchmark_requests_by_status(&[BenchmarkRequestStatus::ArtifactsReady]) @@ -521,7 +537,9 @@ mod tests { ); let mut db = db.connection().await; - db.insert_benchmark_request(&master_benchmark_request).await; + db.insert_benchmark_request(&master_benchmark_request) + .await + .unwrap(); db.update_benchmark_request_status( &master_benchmark_request, @@ -561,7 +579,9 @@ mod tests { "cranelift", "", ); - db.insert_benchmark_request(&try_benchmark_request).await; + db.insert_benchmark_request(&try_benchmark_request) + .await + .unwrap(); db.attach_shas_to_try_benchmark_request(pr, "foo", "bar") .await .unwrap(); @@ -597,7 +617,7 @@ mod tests { "cranelift", "", ); - db.insert_benchmark_request(&completed_try).await; + db.insert_benchmark_request(&completed_try).await.unwrap(); let try_benchmark_request = BenchmarkRequest::create_try( None, @@ -609,9 +629,14 @@ mod tests { "", ); // deliberately insert twice - db.insert_benchmark_request(&try_benchmark_request).await; + db.insert_benchmark_request(&try_benchmark_request) + .await + .unwrap(); // this one should fail - db.insert_benchmark_request(&try_benchmark_request).await; + assert!(db + .insert_benchmark_request(&try_benchmark_request) + .await + .is_err()); db.attach_shas_to_try_benchmark_request(pr, "foo", "bar") .await .unwrap(); diff --git a/database/src/pool/postgres.rs b/database/src/pool/postgres.rs index bf2635738..7db2291b0 100644 --- a/database/src/pool/postgres.rs +++ b/database/src/pool/postgres.rs @@ -1387,7 +1387,10 @@ where .unwrap(); } - async fn insert_benchmark_request(&self, benchmark_request: &BenchmarkRequest) { + async fn insert_benchmark_request( + &self, + benchmark_request: &BenchmarkRequest, + ) -> anyhow::Result<()> { self.conn() .execute( r#" @@ -1401,22 +1404,22 @@ where backends, profiles ) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8) - ON CONFLICT DO NOTHING; + VALUES ($1, $2, $3, $4, $5, $6, $7, $8); "#, &[ &benchmark_request.tag(), &benchmark_request.parent_sha(), &benchmark_request.pr().map(|it| *it as i32), - &benchmark_request.commit_type(), - &benchmark_request.status.to_string(), + &benchmark_request.commit_type, + &benchmark_request.status, &benchmark_request.created_at, &benchmark_request.backends, &benchmark_request.profiles, ], ) .await - .unwrap(); + .context("Failed to insert benchmark request")?; + Ok(()) } async fn get_benchmark_requests_by_status( diff --git a/database/src/pool/sqlite.rs b/database/src/pool/sqlite.rs index b218c0f6d..6dcc9a608 100644 --- a/database/src/pool/sqlite.rs +++ b/database/src/pool/sqlite.rs @@ -1264,7 +1264,10 @@ impl Connection for SqliteConnection { .unwrap(); } - async fn insert_benchmark_request(&self, _benchmark_request: &BenchmarkRequest) { + async fn insert_benchmark_request( + &self, + _benchmark_request: &BenchmarkRequest, + ) -> anyhow::Result<()> { no_queue_implementation_abort!() } diff --git a/site/src/job_queue.rs b/site/src/job_queue.rs index 65f4c6d73..1bde1ae06 100644 --- a/site/src/job_queue.rs +++ b/site/src/job_queue.rs @@ -42,7 +42,9 @@ async fn create_benchmark_request_master_commits( "", "", ); - conn.insert_benchmark_request(&benchmark).await; + if let Err(e) = conn.insert_benchmark_request(&benchmark).await { + log::error!("Failed to insert master benchmark request: {}", e); + } } } Ok(()) @@ -130,7 +132,9 @@ async fn create_benchmark_request_releases( "", "", ); - conn.insert_benchmark_request(&release_request).await; + if let Err(e) = conn.insert_benchmark_request(&release_request).await { + log::error!("Failed to insert release benchmark request: {}", e); + } } } Ok(()) @@ -401,7 +405,7 @@ mod tests { requests: &[BenchmarkRequest], ) { for request in requests { - conn.insert_benchmark_request(&request).await; + conn.insert_benchmark_request(&request).await.unwrap(); } } diff --git a/site/src/request_handlers/github.rs b/site/src/request_handlers/github.rs index f26edf9cc..b3a0aedc2 100644 --- a/site/src/request_handlers/github.rs +++ b/site/src/request_handlers/github.rs @@ -93,7 +93,9 @@ async fn queue_partial_try_benchmark_request( backends, "", ); - conn.insert_benchmark_request(&try_request).await; + if let Err(e) = conn.insert_benchmark_request(&try_request).await { + log::error!("Failed to insert try benchmark request: {}", e); + } } }