From aee35d6d511d16130fb64ae4dd2e28757e99f79a Mon Sep 17 00:00:00 2001 From: Guillaume Bouvignies Date: Fri, 10 Nov 2023 18:31:14 +0100 Subject: [PATCH] fix: Invalid config for workers does not panic (#2612) * fix: Invalid config for workers does not panic * Remove unused import * Fix error type * cleanup unused imports --- backend/src/monitor.rs | 2 +- backend/windmill-common/src/worker.rs | 43 +++++++++++++++++---------- 2 files changed, 28 insertions(+), 17 deletions(-) diff --git a/backend/src/monitor.rs b/backend/src/monitor.rs index 6042b187ca..8f2428c00a 100644 --- a/backend/src/monitor.rs +++ b/backend/src/monitor.rs @@ -525,7 +525,7 @@ pub async fn reload_worker_config( tx: tokio::sync::broadcast::Sender<()>, kill_if_change: bool, ) { - let config = load_worker_config(&db).await; + let config = load_worker_config(&db, tx.clone()).await; if let Err(e) = config { tracing::error!("Error reloading worker config: {:?}", e) } else { diff --git a/backend/windmill-common/src/worker.rs b/backend/windmill-common/src/worker.rs index 3585f8f9b0..5ac3d447c5 100644 --- a/backend/windmill-common/src/worker.rs +++ b/backend/windmill-common/src/worker.rs @@ -1,13 +1,12 @@ +use itertools::Itertools; +use regex::Regex; +use serde::{Deserialize, Serialize}; +use serde_json::value::RawValue; use std::{ cmp::Reverse, collections::{HashMap, HashSet}, sync::Arc, }; - -use itertools::Itertools; -use regex::Regex; -use serde::{Deserialize, Serialize}; -use serde_json::value::RawValue; use tokio::sync::RwLock; use crate::{error, global_settings::CUSTOM_TAGS_SETTING, server::ServerConfig, DB}; @@ -150,7 +149,10 @@ pub async fn update_ping(worker_instance: &str, worker_name: &str, ip: &str, db: .expect("insert worker_ping initial value"); } -pub async fn load_worker_config(db: &DB) -> error::Result { +pub async fn load_worker_config( + db: &DB, + killpill_tx: tokio::sync::broadcast::Sender<()>, +) -> error::Result { tracing::info!("Loading config from WORKER_GROUP: {}", *WORKER_GROUP); let mut config: WorkerConfigOpt = sqlx::query_scalar!( "SELECT config FROM config WHERE name = $1", @@ -178,16 +180,25 @@ pub async fn load_worker_config(db: &DB) -> error::Result { config.dedicated_worker.as_ref().unwrap() ); } - let dedicated_worker = config.dedicated_worker.map(|x| { - let splitted = x.split(':').to_owned().collect_vec(); - if splitted.len() != 2 { - panic!("DEDICATED_WORKER setting should be in the form of :") - } else { - let workspace = splitted[0]; - let script_path = splitted[1]; - WorkspacedPath { workspace_id: workspace.to_string(), path: script_path.to_string() } - } - }); + let dedicated_worker = config + .dedicated_worker + .map(|x| { + let splitted = x.split(':').to_owned().collect_vec(); + if splitted.len() != 2 { + killpill_tx.send(()).expect("send"); + return Err(anyhow::anyhow!( + "Invalid dedicated_worker format. Got {x}, expects :" + )); + } else { + let workspace = splitted[0]; + let script_path = splitted[1]; + Ok(WorkspacedPath { + workspace_id: workspace.to_string(), + path: script_path.to_string(), + }) + } + }) + .transpose()?; if *WORKER_GROUP == "default" && dedicated_worker.is_none() { let mut all_tags = config .worker_tags