Pr Feedback; code tidy up & cron function refactor

Author: Jamesbarford

database/src/lib.rs

@@ -823,13 +823,13 @@ pub enum BenchmarkRequestType {
     /// A Try commit
     Try {
         sha: String,
-        parent_sha: Option<String>,
+        parent_sha: String,
         pr: u32,
     },
     /// A Master commit
     Master {
         sha: String,
-        parent_sha: Option<String>,
+        parent_sha: String,
         pr: u32,
     },
     /// A release only has a tag
@@ -857,16 +857,8 @@ impl BenchmarkRequestType {
 impl fmt::Display for BenchmarkRequestType {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
-            BenchmarkRequestType::Try {
-                sha: _,
-                parent_sha: _,
-                pr: _,
-            } => write!(f, "try"),
-            BenchmarkRequestType::Master {
-                sha: _,
-                parent_sha: _,
-                pr: _,
-            } => write!(f, "master"),
+            BenchmarkRequestType::Try { .. } => write!(f, "try"),
+            BenchmarkRequestType::Master { .. } => write!(f, "master"),
             BenchmarkRequestType::Release { tag: _ } => write!(f, "release"),
         }
     }
@@ -904,7 +896,7 @@ impl BenchmarkRequest {
 
     pub fn create_try(
         sha: &str,
-        parent_sha: Option<&str>,
+        parent_sha: &str,
         pr: u32,
         created_at: DateTime<Utc>,
         status: BenchmarkRequestStatus,
@@ -915,7 +907,7 @@ impl BenchmarkRequest {
             commit_type: BenchmarkRequestType::Try {
                 pr,
                 sha: sha.to_string(),
-                parent_sha: parent_sha.map(|it| it.to_string()),
+                parent_sha: parent_sha.to_string(),
             },
             created_at,
             completed_at: None,
@@ -927,7 +919,7 @@ impl BenchmarkRequest {
 
     pub fn create_master(
         sha: &str,
-        parent_sha: Option<&str>,
+        parent_sha: &str,
         pr: u32,
         created_at: DateTime<Utc>,
         status: BenchmarkRequestStatus,
@@ -938,7 +930,7 @@ impl BenchmarkRequest {
             commit_type: BenchmarkRequestType::Master {
                 pr,
                 sha: sha.to_string(),
-                parent_sha: parent_sha.map(|it| it.to_string()),
+                parent_sha: parent_sha.to_string(),
             },
             created_at,
             completed_at: None,
@@ -952,32 +944,16 @@ impl BenchmarkRequest {
     /// `release`
     pub fn tag(&self) -> &str {
         match &self.commit_type {
-            BenchmarkRequestType::Try {
-                sha,
-                parent_sha: _,
-                pr: _,
-            }
-            | BenchmarkRequestType::Master {
-                sha,
-                parent_sha: _,
-                pr: _,
-            } => sha,
+            BenchmarkRequestType::Try { sha, .. } | BenchmarkRequestType::Master { sha, .. } => sha,
             BenchmarkRequestType::Release { tag } => tag,
         }
     }
 
     pub fn pr(&self) -> Option<&u32> {
         match &self.commit_type {
-            BenchmarkRequestType::Try {
-                sha: _,
-                parent_sha: _,
-                pr,
+            BenchmarkRequestType::Try { pr, .. } | BenchmarkRequestType::Master { pr, .. } => {
+                Some(pr)
             }
-            | BenchmarkRequestType::Master {
-                sha: _,
-                parent_sha: _,
-                pr,
-            } => Some(pr),
             BenchmarkRequestType::Release { tag: _ } => None,
         }
     }
@@ -988,19 +964,8 @@ impl BenchmarkRequest {
 
     pub fn parent_sha(&self) -> Option<&str> {
         match &self.commit_type {
-            BenchmarkRequestType::Try {
-                sha: _,
-                parent_sha,
-                pr: _,
-            }
-            | BenchmarkRequestType::Master {
-                sha: _,
-                parent_sha,
-                pr: _,
-            } => match parent_sha {
-                Some(parent_sha) => Some(parent_sha),
-                None => None,
-            },
+            BenchmarkRequestType::Try { parent_sha, .. }
+            | BenchmarkRequestType::Master { parent_sha, .. } => Some(parent_sha),
             BenchmarkRequestType::Release { tag: _ } => None,
         }
     }

database/src/pool.rs

@@ -386,7 +386,7 @@ mod tests {
             let time = chrono::DateTime::from_str("2021-09-01T00:00:00.000Z").unwrap();
             let master_benchmark_request = BenchmarkRequest::create_master(
                 "a-sha-1",
-                Some("parent-sha-1"),
+                "parent-sha-1",
                 42,
                 time,
                 BenchmarkRequestStatus::WaitingForParent,
@@ -396,7 +396,7 @@ mod tests {
 
             let try_benchmark_request = BenchmarkRequest::create_try(
                 "b-sha-2",
-                Some("parent-sha-2"),
+                "parent-sha-2",
                 32,
                 time,
                 BenchmarkRequestStatus::WaitingForParent,

site/src/job_queue.rs

@@ -6,54 +6,52 @@ use database::{BenchmarkRequest, BenchmarkRequestStatus};
 use parking_lot::RwLock;
 use tokio::time::{self, Duration};
 
-impl SiteCtxt {
-    /// Store the latest master commits or do nothing if all of them are
-    /// already in the database
-    pub async fn enqueue_master_commits(&self) {
-        let conn = self.conn().await;
-        let master_commits = &self.get_master_commits().commits;
-        // TODO; delete at some point in the future
-        let cutoff: chrono::DateTime<Utc> =
-            chrono::DateTime::from_str("2025-06-01T00:00:00.000Z").unwrap();
+/// Store the latest master commits or do nothing if all of them are
+/// already in the database
+async fn enqueue_master_commits(ctxt: &Arc<SiteCtxt>) {
+    let conn = ctxt.conn().await;
+    let master_commits = &ctxt.get_master_commits().commits;
+    // TODO; delete at some point in the future
+    let cutoff: chrono::DateTime<Utc> =
+        chrono::DateTime::from_str("2025-06-01T00:00:00.000Z").unwrap();
 
-        for master_commit in master_commits {
-            // We don't want to add masses of obsolete data
-            if master_commit.time >= cutoff {
-                let pr = master_commit.pr.unwrap_or(0);
-                let benchmark = BenchmarkRequest::create_master(
-                    &master_commit.sha,
-                    Some(&master_commit.parent_sha),
-                    pr,
-                    master_commit.time,
-                    BenchmarkRequestStatus::WaitingForParent,
-                    "",
-                    "",
-                );
-                conn.insert_benchmark_request(&benchmark).await;
-            }
+    for master_commit in master_commits {
+        // We don't want to add masses of obsolete data
+        if master_commit.time >= cutoff {
+            let pr = master_commit.pr.unwrap_or(0);
+            let benchmark = BenchmarkRequest::create_master(
+                &master_commit.sha,
+                &master_commit.parent_sha,
+                pr,
+                master_commit.time,
+                BenchmarkRequestStatus::WaitingForParent,
+                "",
+                "",
+            );
+            conn.insert_benchmark_request(&benchmark).await;
         }
     }
 }
 
-/// Inserts and manages the queue at `seconds` interval
-async fn cron_enqueue_jobs(site_ctxt: Arc<RwLock<Option<Arc<SiteCtxt>>>>, seconds: u64) {
-    let mut interval = time::interval(Duration::from_secs(seconds));
-
-    loop {
-        let ctxt = site_ctxt.clone();
-        if let Some(ctxt_clone) = {
-            let guard = ctxt.read();
-            guard.as_ref().cloned()
-        } {
-            ctxt_clone.enqueue_master_commits().await;
-        }
-
-        interval.tick().await;
-        println!("Cron job executed at: {:?}", std::time::SystemTime::now());
-    }
+/// For queueing jobs, add the jobs you want to queue to this function
+async fn cron_enqueue_jobs(site_ctxt: &Arc<SiteCtxt>) {
+    // Put the master commits into the `benchmark_requests` queue
+    enqueue_master_commits(&site_ctxt).await;
 }
 
 /// Entry point for the cron
 pub async fn cron_main(site_ctxt: Arc<RwLock<Option<Arc<SiteCtxt>>>>, seconds: u64) {
-    cron_enqueue_jobs(site_ctxt, seconds).await;
+    let ctxt = site_ctxt.clone();
+    let mut interval = time::interval(Duration::from_secs(seconds));
+
+    if let Some(ctxt_clone) = {
+        let guard = ctxt.read();
+        guard.as_ref().cloned()
+    } {
+        loop {
+            cron_enqueue_jobs(&ctxt_clone).await;
+            interval.tick().await;
+            println!("Cron job executed at: {:?}", std::time::SystemTime::now());
+        }
+    }
 }