Merge pull request #1267 from rylev/fix-get-error

Fix `get_error` query

Author: rylev

database/src/pool.rs

@@ -104,8 +104,7 @@ pub trait Connection: Send + Sync {
         series: u32,
         artifact_row_id: ArtifactIdNumber,
     ) -> Option<QueryDatum>;
-    async fn get_error(&self, artifact_row_id: ArtifactIdNumber)
-        -> HashMap<String, Option<String>>;
+    async fn get_error(&self, artifact_row_id: ArtifactIdNumber) -> HashMap<String, String>;
 
     async fn queue_pr(
         &self,

database/src/pool/postgres.rs

@@ -430,7 +430,7 @@ impl PostgresConnection {
                     .await
                     .unwrap(),
                 get_error: conn.prepare("select crate, error from error_series
-                    left join error on error.series = error_series.id and aid = $1").await.unwrap(),
+                    inner join error on error.series = error_series.id and aid = $1").await.unwrap(),
                 select_self_query_series: conn.prepare("select id from self_profile_query_series where crate = $1 and profile = $2 and cache = $3 and query = $4").await.unwrap(),
                 insert_self_query_series: conn.prepare("insert into self_profile_query_series (crate, profile, cache, query) VALUES ($1, $2, $3, $4) ON CONFLICT DO NOTHING RETURNING id").await.unwrap(),
                 insert_pstat_series: conn.prepare("insert into pstat_series (crate, profile, cache, statistic) VALUES ($1, $2, $3, $4) ON CONFLICT DO NOTHING RETURNING id").await.unwrap(),
@@ -703,10 +703,7 @@ where
             })
             .collect()
     }
-    async fn get_error(
-        &self,
-        artifact_row_id: crate::ArtifactIdNumber,
-    ) -> HashMap<String, Option<String>> {
+    async fn get_error(&self, artifact_row_id: crate::ArtifactIdNumber) -> HashMap<String, String> {
         let rows = self
             .conn()
             .query(&self.statements().get_error, &[&(artifact_row_id.0 as i32)])

database/src/pool/sqlite.rs

@@ -620,11 +620,11 @@ impl Connection for SqliteConnection {
             .collect::<Result<_, _>>()
             .unwrap()
     }
-    async fn get_error(&self, aid: crate::ArtifactIdNumber) -> HashMap<String, Option<String>> {
+    async fn get_error(&self, aid: crate::ArtifactIdNumber) -> HashMap<String, String> {
         self.raw_ref()
             .prepare_cached(
                 "select crate, error from error_series
-                    left join error on error.series = error_series.id and aid = ?",
+                    inner join error on error.series = error_series.id and aid = ?",
             )
             .unwrap()
             .query_map(params![&aid.0], |row| Ok((row.get(0)?, row.get(1)?)))

site/src/api.rs

@@ -206,8 +206,7 @@ pub mod status {
     #[derive(Debug, PartialEq, Clone, Serialize, Deserialize)]
     pub struct BenchmarkStatus {
         pub name: String,
-        pub success: bool,
-        pub error: Option<String>,
+        pub error: String,
     }
 
     #[derive(Serialize, Debug)]

site/src/comparison.rs

@@ -622,21 +622,17 @@ async fn compare_given_commits(
         })
         .collect();
 
-    let mut errors = conn.get_error(b.lookup(&idx).unwrap()).await;
-    for (name, error) in conn.get_error(a.lookup(&idx).unwrap()).await {
-        if error.is_some() {
-            errors.remove(&name);
-        }
+    let mut errors_in_b = conn.get_error(b.lookup(&idx).unwrap()).await;
+    let errors_in_a = conn.get_error(a.lookup(&idx).unwrap()).await;
+    for (name, _) in errors_in_a {
+        errors_in_b.remove(&name);
     }
 
     Ok(Some(Comparison {
         a: ArtifactDescription::for_artifact(&*conn, a.clone(), master_commits).await,
         b: ArtifactDescription::for_artifact(&*conn, b.clone(), master_commits).await,
         statistics,
-        new_errors: errors
-            .into_iter()
-            .filter_map(|(k, v)| v.map(|v| (k, v)))
-            .collect(),
+        new_errors: errors_in_b.into_iter().collect(),
     }))
 }
 

site/src/request_handlers/status_page.rs

@@ -41,25 +41,14 @@ pub async fn handle_status_page(ctxt: Arc<SiteCtxt>) -> status::Response {
     } else {
         Default::default()
     };
-    let mut benchmark_state = errors
+    let benchmark_state = errors
         .into_iter()
         .map(|(name, error)| {
-            let msg = if let Some(error) = error {
-                Some(prettify_log(&error).unwrap_or(error))
-            } else {
-                None
-            };
-            status::BenchmarkStatus {
-                name,
-                success: msg.is_none(),
-                error: msg,
-            }
+            let error = prettify_log(&error).unwrap_or(error);
+            status::BenchmarkStatus { name, error }
         })
         .collect::<Vec<_>>();
 
-    benchmark_state.sort_by_key(|s| s.error.is_some());
-    benchmark_state.reverse();
-
     status::Response {
         last_commit,
         benchmarks: benchmark_state,

site/static/status.html

@@ -27,7 +27,7 @@
     </div>
     <div id="data">
         <div id="data-insert-js"></div>
-        Benchmarks for last commit:
+        Benchmarking errors for last commit:
         <div id="benchmark-state"></div>
     </div>
     <div id="as-of"></div>
@@ -46,19 +46,11 @@
             }
             for (let benchmark of data.benchmarks) {
                 let element = document.createElement("div");
-                if (benchmark.error) {
-                    element.innerHTML = `<details open>
+                element.innerHTML = `<details open>
                     <summary>${benchmark.name} - error</summary>
                     <pre class="benchmark-error"></pre>
                 </details>`;
-                } else {
-                    element.innerHTML = `
-                    <p style="margin:0.1em;">${benchmark.name} - successful</p>
-                `;
-                }
-                if (benchmark.error) {
-                    element.querySelector(".benchmark-error").innerText = benchmark.error;
-                }
+                element.querySelector(".benchmark-error").innerText = benchmark.error;
                 state_div.appendChild(element);
             }
             let missing_div = document.querySelector("#data-insert-js");
@@ -175,8 +167,7 @@
                 return `${reason_to_string(reason.InProgress)} - in progress`;
             } else if (reason["Master"] != undefined && reason.Master.pr) {
                 return `<a href="https://github.com/rust-lang/rust/pull/${reason["Master"].pr}">
-                    #${reason["Master"].pr}</a>${
-                    reason.Master.is_try_parent ? " - Try commit parent" : ""
+                    #${reason["Master"].pr}</a>${reason.Master.is_try_parent ? " - Try commit parent" : ""
                     }`;
             } else if (reason["Master"] != undefined && reason.Master.pr == 0) {
                 return "Master";
@@ -227,4 +218,4 @@
     </script>
 </body>
 
-</html>
+</html>