Chore; make tag optional for Try benchmark requests

Author: Jamesbarford

database/src/lib.rs

@@ -842,7 +842,7 @@ impl<'a> tokio_postgres::types::FromSql<'a> for BenchmarkRequestStatus {
 pub enum BenchmarkRequestType {
     /// A Try commit
     Try {
-        sha: String,
+        sha: Option<String>,
         parent_sha: Option<String>,
         pr: u32,
     },
@@ -915,7 +915,7 @@ impl BenchmarkRequest {
     }
 
     pub fn create_try(
-        sha: &str,
+        sha: Option<&str>,
         parent_sha: Option<&str>,
         pr: u32,
         created_at: DateTime<Utc>,
@@ -926,7 +926,7 @@ impl BenchmarkRequest {
         Self {
             commit_type: BenchmarkRequestType::Try {
                 pr,
-                sha: sha.to_string(),
+                sha: sha.map(|it| it.to_string()),
                 parent_sha: parent_sha.map(|it| it.to_string()),
             },
             created_at,
@@ -962,10 +962,11 @@ impl BenchmarkRequest {
 
     /// Get either the `sha` for a `try` or `master` commit or a `tag` for a
     /// `release`
-    pub fn tag(&self) -> &str {
+    pub fn tag(&self) -> Option<&str> {
         match &self.commit_type {
-            BenchmarkRequestType::Try { sha, .. } | BenchmarkRequestType::Master { sha, .. } => sha,
-            BenchmarkRequestType::Release { tag } => tag,
+            BenchmarkRequestType::Try { sha, .. } => sha.as_deref(),
+            BenchmarkRequestType::Master { sha, .. } => Some(sha),
+            BenchmarkRequestType::Release { tag } => Some(tag),
         }
     }
 

database/src/pool.rs

@@ -409,7 +409,7 @@ mod tests {
             );
 
             let try_benchmark_request = BenchmarkRequest::create_try(
-                "b-sha-2",
+                Some("b-sha-2"),
                 Some("parent-sha-2"),
                 32,
                 time,
@@ -457,7 +457,7 @@ mod tests {
             );
 
             let try_benchmark_request = BenchmarkRequest::create_try(
-                "b-sha-2",
+                Some("b-sha-2"),
                 Some("parent-sha-2"),
                 32,
                 time,

database/src/pool/postgres.rs

@@ -301,6 +301,10 @@ static MIGRATIONS: &[&str] = &[
     );
     CREATE INDEX IF NOT EXISTS benchmark_request_status_idx on benchmark_request (status) WHERE status != 'completed';
     "#,
+    // Remove that tag cannot be NULL
+    r#"ALTER TABLE benchmark_request ALTER COLUMN tag DROP NOT NULL;"#,
+    // Prevent multiple try commits without a `sha` being added to the table
+    r#"CREATE UNIQUE INDEX benchmark_request_pr_status_idx ON benchmark_request (pr, commit_type) WHERE tag IS NULL;"#,
 ];
 
 #[async_trait::async_trait]
@@ -1457,7 +1461,7 @@ where
                 match commit_type {
                     "try" => {
                         let mut try_benchmark = BenchmarkRequest::create_try(
-                            tag,
+                            Some(tag),
                             parent_sha,
                             pr.unwrap() as u32,
                             created_at,

site/src/job_queue.rs

@@ -182,7 +182,9 @@ fn sort_benchmark_requests(done: &HashSet<String>, request_queue: &mut [Benchmar
             )
         });
         for c in level {
-            done.insert(c.tag().to_string());
+            if let Some(tag) = c.tag() {
+                done.insert(tag.to_string());
+            }
         }
         finished += level_len;
     }
@@ -241,7 +243,7 @@ pub async fn build_queue(
 
     release_artifacts.sort_unstable_by(|a, b| {
         a.tag()
-            .cmp(b.tag())
+            .cmp(&b.tag())
             .then_with(|| a.created_at.cmp(&b.created_at))
     });
 
@@ -258,7 +260,7 @@ async fn enqueue_next_job(conn: &mut dyn database::pool::Connection) -> anyhow::
         .get_benchmark_requests_by_status(&[BenchmarkRequestStatus::Completed])
         .await?
         .into_iter()
-        .map(|request| request.tag().to_string())
+        .filter_map(|request| request.tag().map(|tag| tag.to_string()))
         .collect();
 
     let queue = build_queue(conn, &completed).await?;
@@ -360,7 +362,7 @@ mod tests {
 
     fn create_try(sha: &str, parent: &str, pr: u32, age_days: &str) -> BenchmarkRequest {
         BenchmarkRequest::create_try(
-            sha,
+            Some(sha),
             Some(parent),
             pr,
             days_ago(age_days),
@@ -390,7 +392,10 @@ mod tests {
     }
 
     fn queue_order_matches(queue: &[BenchmarkRequest], expected: &[&str]) {
-        let queue_shas: Vec<&str> = queue.iter().map(|req| req.tag()).collect();
+        let queue_shas: Vec<&str> = queue
+            .iter()
+            .filter_map(|request| request.tag().map(|tag| tag))
+            .collect();
         assert_eq!(queue_shas, expected)
     }