PR Feedback: rename enum variants and update messages

Author: Jamesbarford

database/src/lib.rs

@@ -1041,8 +1041,8 @@ impl BenchmarkRequest {
 
 #[derive(Debug, Clone, PartialEq)]
 pub enum BenchmarkRequestInsertResult {
-    Queued,
-    AlreadyQueued,
+    Inserted,
+    NothingInserted,
 }
 
 pub fn parse_backends(backends: &str) -> Result<Vec<CodegenBackend>, String> {

database/src/pool.rs

@@ -543,7 +543,10 @@ mod tests {
                 .await;
 
             assert!(result.is_ok());
-            assert_eq!(result.unwrap(), BenchmarkRequestInsertResult::AlreadyQueued);
+            assert_eq!(
+                result.unwrap(),
+                BenchmarkRequestInsertResult::NothingInserted
+            );
 
             Ok(ctx)
         })
@@ -576,7 +579,7 @@ mod tests {
                 .await
                 .unwrap();
 
-            assert_eq!(result, BenchmarkRequestInsertResult::AlreadyQueued);
+            assert_eq!(result, BenchmarkRequestInsertResult::NothingInserted);
             Ok(ctx)
         })
         .await;
@@ -607,7 +610,7 @@ mod tests {
                 .await
                 .unwrap();
 
-            assert_eq!(result, BenchmarkRequestInsertResult::AlreadyQueued);
+            assert_eq!(result, BenchmarkRequestInsertResult::NothingInserted);
 
             Ok(ctx)
         })

database/src/pool/postgres.rs

@@ -1670,9 +1670,9 @@ where
         if rows.is_empty() {
             // Allows us to handle duplicated cases without the database auto
             // erroring
-            Ok(BenchmarkRequestInsertResult::AlreadyQueued)
+            Ok(BenchmarkRequestInsertResult::NothingInserted)
         } else {
-            Ok(BenchmarkRequestInsertResult::Queued)
+            Ok(BenchmarkRequestInsertResult::Inserted)
         }
     }
 

site/src/job_queue/mod.rs

@@ -56,12 +56,12 @@ async fn create_benchmark_request_master_commits(
             log::info!("Inserting master benchmark request {benchmark:?}");
 
             match conn.insert_benchmark_request(&benchmark).await {
-                Ok(BenchmarkRequestInsertResult::AlreadyQueued) => {
+                Ok(BenchmarkRequestInsertResult::NothingInserted) => {
                     log::error!(
                         "Failed to insert master benchmark request, request for PR`#{pr}` already exists",
                     );
                 }
-                Ok(BenchmarkRequestInsertResult::Queued) => {}
+                Ok(BenchmarkRequestInsertResult::Inserted) => {}
                 Err(e) => {
                     log::error!("Failed to insert master benchmark request: {e:?}");
                 }
@@ -95,12 +95,12 @@ async fn create_benchmark_request_releases(
             log::info!("Inserting release benchmark request {release_request:?}");
 
             match conn.insert_benchmark_request(&release_request).await {
-                Ok(BenchmarkRequestInsertResult::AlreadyQueued) => {
+                Ok(BenchmarkRequestInsertResult::NothingInserted) => {
                     log::error!(
                         "Failed to insert release benchmark request, release with tag `{name}` already exists"
                     );
                 }
-                Ok(BenchmarkRequestInsertResult::Queued) => {}
+                Ok(BenchmarkRequestInsertResult::Inserted) => {}
                 Err(e) => {
                     log::error!("Failed to insert release benchmark request: {e}");
                 }

site/src/request_handlers/github.rs

@@ -90,34 +90,26 @@ fn get_awaiting_on_bors_message() -> String {
 /// of this commit existing. The DB ensures that there is only one non-completed
 /// try benchmark request per `pr`.
 async fn record_try_benchmark_request_without_artifacts(
-    client: &client::Client,
     conn: &dyn database::pool::Connection,
     pr: u32,
     backends: &str,
-) -> ServerResult<github::Response> {
+) -> String {
     let try_request = BenchmarkRequest::create_try_without_artifacts(pr, backends, "");
     log::info!("Inserting try benchmark request {try_request:?}");
 
     match conn.insert_benchmark_request(&try_request).await {
-        Ok(BenchmarkRequestInsertResult::AlreadyQueued) => {
-            log::error!(
+        Ok(BenchmarkRequestInsertResult::NothingInserted) => {
+            log::info!(
                 "Failed to insert try benchmark request, a request for PR`#{pr}` already exists"
             );
-            client.post_comment(pr, format!("Failed to insert try benchmark request, a request for PR`#{pr}` already exists")).await;
-        }
-        Ok(BenchmarkRequestInsertResult::Queued) => {
-            client
-                .post_comment(pr, get_awaiting_on_bors_message())
-                .await;
+            format!("This pull request was already queued before and is awaiting a try build to finish.")
         }
+        Ok(BenchmarkRequestInsertResult::Inserted) => get_awaiting_on_bors_message(),
         Err(e) => {
             log::error!("Failed to insert release benchmark request: {e}");
-            client
-                .post_comment(pr, "Something went wrong! This is most likely an internal failure, please message on Zulip".to_string())
-                .await;
+            "Something went wrong! This is most likely an internal failure, please let us know on [Zulip](https://rust-lang.zulipchat.com/#narrow/channel/247081-t-compiler.2Fperformance)".to_string()
         }
     }
-    Ok(github::Response)
 }
 
 async fn handle_rust_timer(
@@ -152,13 +144,12 @@ async fn handle_rust_timer(
                 let conn = ctxt.conn().await;
 
                 if should_use_job_queue(issue.number) {
-                    return record_try_benchmark_request_without_artifacts(
-                        main_client,
+                    record_try_benchmark_request_without_artifacts(
                         &*conn,
                         issue.number,
                         cmd.params.backends.unwrap_or(""),
                     )
-                    .await;
+                    .await
                 } else {
                     conn.queue_pr(
                         issue.number,
@@ -168,8 +159,8 @@ async fn handle_rust_timer(
                         cmd.params.backends,
                     )
                     .await;
+                    get_awaiting_on_bors_message()
                 }
-                get_awaiting_on_bors_message()
             }
             Err(error) => {
                 format!("Error occurred while parsing comment: {error}")
@@ -201,8 +192,7 @@ async fn handle_rust_timer(
         let conn = ctxt.conn().await;
         for command in &valid_build_cmds {
             if should_use_job_queue(issue.number) {
-                return record_try_benchmark_request_without_artifacts(
-                    main_client,
+                record_try_benchmark_request_without_artifacts(
                     &*conn,
                     issue.number,
                     command.params.backends.unwrap_or(""),