Do not send "commit queued" comments after every try build completes

Author: Kobzol

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<Utc>,
-    ) -> anyhow::Result<()>;
+    ) -> anyhow::Result<bool>;
 
     /// 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 {

database/src/pool/postgres.rs

@@ -1717,21 +1717,21 @@ where
         sha: &str,
         parent_sha: &str,
         commit_date: DateTime<Utc>,
-    ) -> anyhow::Result<()> {
-        self.conn()
+    ) -> anyhow::Result<bool> {
+        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<PendingBenchmarkRequests> {

database/src/pool/sqlite.rs

@@ -1344,7 +1344,7 @@ impl Connection for SqliteConnection {
         _sha: &str,
         _parent_sha: &str,
         _commit_date: DateTime<Utc>,
-    ) -> anyhow::Result<()> {
+    ) -> anyhow::Result<bool> {
         no_queue_implementation_abort!()
     }
 

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<Utc>,
-) {
-    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,