Auto merge of #706 - Mark-Simulacrum:no-state, r=Mark-Simulacrum

bors · bors · commit 00ec081447d8 · 2023-10-29T19:59:28.000Z
Remove a few unused internal details

See individual commits for details.
diff --git a/src/agent/api.rs b/src/agent/api.rs
@@ -182,14 +182,12 @@ impl AgentApi {
                 .build_request(Method::POST, "record-progress")
                 .json(&json!({
                     "experiment-name": ex.name,
-                    "results": [
-                        {
-                            "crate": krate,
-                            "toolchain": toolchain,
-                            "result": result,
-                            "log": base64::engine::general_purpose::STANDARD.encode(log),
-                        },
-                    ],
+                    "result": {
+                        "crate": krate,
+                        "toolchain": toolchain,
+                        "result": result,
+                        "log": base64::engine::general_purpose::STANDARD.encode(log),
+                    },
                     "version": version
                 }))
                 .send()?
diff --git a/src/results/db.rs b/src/results/db.rs
@@ -20,7 +20,7 @@ pub struct TaskResult {
 
 #[derive(Deserialize)]
 pub struct ProgressData {
-    pub results: Vec<TaskResult>,
+    pub result: TaskResult,
     pub version: Option<(Crate, Crate)>,
 }
 
@@ -83,25 +83,23 @@ impl<'a> DatabaseDB<'a> {
         data: &ProgressData,
         encoding_type: EncodingType,
     ) -> Fallible<()> {
-        for result in &data.results {
-            self.store_result(
-                ex,
-                &result.krate,
-                &result.toolchain,
-                &result.result,
-                &base64::engine::general_purpose::STANDARD
-                    .decode(&result.log)
-                    .with_context(|_| "invalid base64 log provided")?,
-                encoding_type,
-            )?;
-
-            if let Some((old, new)) = &data.version {
-                self.update_crate_version(ex, old, new)?;
-            }
-
-            self.mark_crate_as_completed(ex, &result.krate)?;
+        self.store_result(
+            ex,
+            &data.result.krate,
+            &data.result.toolchain,
+            &data.result.result,
+            &base64::engine::general_purpose::STANDARD
+                .decode(&data.result.log)
+                .with_context(|_| "invalid base64 log provided")?,
+            encoding_type,
+        )?;
+
+        if let Some((old, new)) = &data.version {
+            self.update_crate_version(ex, old, new)?;
         }
 
+        self.mark_crate_as_completed(ex, &data.result.krate)?;
+
         Ok(())
     }
 
@@ -465,12 +463,12 @@ mod tests {
             .store(
                 &ex,
                 &ProgressData {
-                    results: vec![TaskResult {
+                    result: TaskResult {
                         krate: updated.clone(),
                         toolchain: MAIN_TOOLCHAIN.clone(),
                         result: TestResult::TestPass,
                         log: base64::engine::general_purpose::STANDARD.encode("foo"),
-                    }],
+                    },
                     version: Some((krate.clone(), updated.clone())),
                 },
                 EncodingType::Plain,
diff --git a/src/runner/tasks.rs b/src/runner/tasks.rs
@@ -48,7 +48,6 @@ impl<'ctx, DB: WriteResults + 'ctx> TaskCtx<'ctx, DB> {
 
 pub(super) enum TaskStep {
     Prepare,
-    Cleanup,
     Skip { tc: Toolchain },
     BuildAndTest { tc: Toolchain, quiet: bool },
     BuildOnly { tc: Toolchain, quiet: bool },
@@ -62,7 +61,6 @@ impl fmt::Debug for TaskStep {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         let (name, quiet, tc) = match *self {
             TaskStep::Prepare => ("prepare", false, None),
-            TaskStep::Cleanup => ("cleanup", false, None),
             TaskStep::Skip { ref tc } => ("skip", false, Some(tc)),
             TaskStep::BuildAndTest { ref tc, quiet } => ("build and test", quiet, Some(tc)),
             TaskStep::BuildOnly { ref tc, quiet } => ("build", quiet, Some(tc)),
@@ -104,7 +102,7 @@ impl Task {
         storage: &LogStorage,
     ) -> Fallible<()> {
         match self.step {
-            TaskStep::Prepare | TaskStep::Cleanup => {}
+            TaskStep::Prepare => {}
             TaskStep::Skip { ref tc }
             | TaskStep::BuildAndTest { ref tc, .. }
             | TaskStep::BuildOnly { ref tc, .. }
@@ -165,10 +163,6 @@ impl Task {
                 tc,
                 false,
             ),
-            TaskStep::Cleanup => {
-                // Remove stored logs
-                return Ok(());
-            }
             TaskStep::Prepare => {
                 logging::capture(logs, || {
                     let rustwide_crate = self.krate.to_rustwide();
diff --git a/src/runner/worker.rs b/src/runner/worker.rs
@@ -91,7 +91,7 @@ impl<'a, DB: WriteResults + Sync> Worker<'a, DB> {
             let mut should_retry = false;
             if res.is_err() && self.ex.toolchains.len() == 2 {
                 let toolchain = match &task.step {
-                    TaskStep::Prepare | TaskStep::Cleanup => None,
+                    TaskStep::Prepare => None,
                     TaskStep::Skip { tc }
                     | TaskStep::BuildAndTest { tc, .. }
                     | TaskStep::BuildOnly { tc, .. }
@@ -200,10 +200,6 @@ impl<'a, DB: WriteResults + Sync> Worker<'a, DB> {
                         },
                     });
                 }
-                tasks.push(Task {
-                    krate: krate.clone(),
-                    step: TaskStep::Cleanup,
-                });
             }
 
             let mut result = Ok(());
diff --git a/src/server/auth.rs b/src/server/auth.rs
@@ -14,11 +14,6 @@ lazy_static! {
         Regex::new(r"^crater(-agent)?/(?P<sha>[a-f0-9]{7,40})( \(.*\))?$").unwrap();
 }
 
-#[derive(Copy, Clone)]
-pub enum TokenType {
-    Agent,
-}
-
 pub struct AuthDetails {
     pub name: String,
     pub git_revision: Option<String>,
@@ -45,7 +40,7 @@ fn git_revision(user_agent: &str) -> Option<String> {
         .map(|cap| cap["sha"].to_string())
 }
 
-fn check_auth(data: &Data, headers: &HeaderMap, token_type: TokenType) -> Option<AuthDetails> {
+fn check_auth(data: &Data, headers: &HeaderMap) -> Option<AuthDetails> {
     // Try to extract the git revision from the User-Agent header
     let git_revision = if let Some(ua_value) = headers.get(USER_AGENT) {
         if let Ok(ua) = ua_value.to_str() {
@@ -60,11 +55,7 @@ fn check_auth(data: &Data, headers: &HeaderMap, token_type: TokenType) -> Option
     if let Some(authorization_value) = headers.get(AUTHORIZATION) {
         if let Ok(authorization) = authorization_value.to_str() {
             if let Some(token) = parse_token(authorization) {
-                let tokens = match token_type {
-                    TokenType::Agent => &data.tokens.agents,
-                };
-
-                if let Some(name) = tokens.get(token) {
+                if let Some(name) = data.tokens.agents.get(token) {
                     return Some(AuthDetails {
                         name: name.clone(),
                         git_revision,
@@ -79,12 +70,11 @@ fn check_auth(data: &Data, headers: &HeaderMap, token_type: TokenType) -> Option
 
 pub fn auth_filter(
     data: Arc<Data>,
-    token_type: TokenType,
 ) -> impl Filter<Extract = (AuthDetails,), Error = Rejection> + Clone {
     warp::header::headers_cloned().and_then(move |headers| {
         let data = data.clone();
         async move {
-            match check_auth(&data, &headers, token_type) {
+            match check_auth(&data, &headers) {
                 Some(details) => Ok(details),
                 None => Err(warp::reject::custom(HttpError::Forbidden)),
             }
diff --git a/src/server/metrics.rs b/src/server/metrics.rs
@@ -3,7 +3,6 @@ use crate::experiments::{Assignee, Experiment};
 use crate::prelude::*;
 use crate::server::agents::Agent;
 use chrono::{DateTime, Utc};
-use prometheus::proto::{Metric, MetricFamily};
 use prometheus::{HistogramVec, IntCounter, IntCounterVec, IntGauge, IntGaugeVec};
 
 const JOBS_METRIC: &str = "crater_completed_jobs_total";
@@ -26,7 +25,7 @@ impl Metrics {
     pub fn new() -> Fallible<Self> {
         let jobs_opts = prometheus::opts!(JOBS_METRIC, "total completed jobs");
         let crater_completed_jobs_total =
-            prometheus::register_int_counter_vec!(jobs_opts, &["agent", "experiment"])?;
+            prometheus::register_int_counter_vec!(jobs_opts, &["experiment"])?;
         let crater_bounced_record_progress = prometheus::register_int_counter!(
             "crater_bounced_record_progress",
             "hits with full record progress queue"
@@ -63,39 +62,15 @@ impl Metrics {
             .inc_by(1);
     }
 
-    pub fn record_completed_jobs(&self, agent: &str, experiment: &str, amount: u64) {
+    pub fn record_completed_jobs(&self, experiment: &str, amount: u64) {
         self.crater_completed_jobs_total
-            .with_label_values(&[agent, experiment])
+            .with_label_values(&[experiment])
             .inc_by(amount);
     }
 
-    fn get_metric_by_name(name: &str) -> Option<MetricFamily> {
-        let families = prometheus::gather();
-        families.into_iter().find(|fam| fam.get_name() == name)
-    }
-
-    fn get_label_by_name<'a>(metric: &'a Metric, label: &str) -> Option<&'a str> {
-        metric
-            .get_label()
-            .iter()
-            .find(|lab| lab.get_name() == label)
-            .map(|lab| lab.get_value())
-    }
-
     fn remove_experiment_jobs(&self, experiment: &str) -> Fallible<()> {
-        if let Some(metric) = Self::get_metric_by_name(JOBS_METRIC) {
-            let agents = metric
-                .get_metric()
-                .iter()
-                .filter(|met| Self::get_label_by_name(met, "experiment").unwrap() == experiment)
-                .map(|met| Self::get_label_by_name(met, "agent").unwrap())
-                .collect::<Vec<&str>>();
-
-            for agent in agents.iter() {
-                self.crater_completed_jobs_total
-                    .remove_label_values(&[agent, experiment])?;
-            }
-        }
+        self.crater_completed_jobs_total
+            .remove_label_values(&[experiment])?;
 
         Ok(())
     }
@@ -143,12 +118,27 @@ mod tests {
     use crate::server::tokens::Tokens;
     use chrono::Utc;
     use lazy_static::lazy_static;
-    use prometheus::proto::MetricFamily;
+    use prometheus::proto::{Metric, MetricFamily};
 
     lazy_static! {
         static ref METRICS: Metrics = Metrics::new().unwrap();
     }
 
+    impl Metrics {
+        fn get_metric_by_name(name: &str) -> Option<MetricFamily> {
+            let families = prometheus::gather();
+            families.into_iter().find(|fam| fam.get_name() == name)
+        }
+
+        fn get_label_by_name<'a>(metric: &'a Metric, label: &str) -> Option<&'a str> {
+            metric
+                .get_label()
+                .iter()
+                .find(|lab| lab.get_name() == label)
+                .map(|lab| lab.get_value())
+        }
+    }
+
     fn test_experiment_presence(metric: &MetricFamily, experiment: &str) -> bool {
         metric
             .get_metric()
@@ -160,12 +150,9 @@ mod tests {
     fn test_on_complete_experiment() {
         let ex1 = "pr-0";
         let ex2 = "pr-1";
-        let agent1 = "agent-1";
-        let agent2 = "agent-2";
 
-        METRICS.record_completed_jobs(agent1, ex1, 1);
-        METRICS.record_completed_jobs(agent2, ex1, 1);
-        METRICS.record_completed_jobs(agent2, ex2, 1);
+        METRICS.record_completed_jobs(ex1, 1);
+        METRICS.record_completed_jobs(ex2, 1);
 
         //test metrics are correctly registered
         let jobs = Metrics::get_metric_by_name(JOBS_METRIC).unwrap();
diff --git a/src/server/routes/agent.rs b/src/server/routes/agent.rs
