fix: do not track relative deps for scripts with raw defined deps from CLI (#5696)

This commit is contained in:
Ruben Fiszel
2025-05-02 00:37:28 +02:00
committed by GitHub
parent bc3bdcb49a
commit ed6313258b
7 changed files with 92 additions and 26 deletions
@@ -1,6 +1,6 @@
{
"db_name": "PostgreSQL",
"query": "INSERT INTO log_file (hostname, mode, worker_group, log_ts, file_path, ok_lines, err_lines, json_fmt) VALUES ($1, $2::text::LOG_MODE, $3, $4, $5, $6, $7, $8)",
"query": "INSERT INTO log_file (hostname, mode, worker_group, log_ts, file_path, ok_lines, err_lines, json_fmt)\n VALUES ($1, $2::text::LOG_MODE, $3, $4, $5, $6, $7, $8)\n ON CONFLICT (hostname, log_ts) DO UPDATE SET ok_lines = log_file.ok_lines + $6, err_lines = log_file.err_lines + $7",
"describe": {
"columns": [],
"parameters": {
@@ -17,5 +17,5 @@
},
"nullable": []
},
"hash": "33c1793e55b1127d88d2509aadd0eb04e042463200f237b4c2cb176612fa16fe"
"hash": "92faee8714a45a403b623e04d789f7f99067a05e9dfe270223164db8a1df2e4b"
}
+3 -1
View File
@@ -638,7 +638,9 @@ async fn send_log_file_to_object_store(
let (ok_lines, err_lines) = read_log_counters(ts_str);
if let Some(db) = conn.as_sql() {
if let Err(e) = sqlx::query!("INSERT INTO log_file (hostname, mode, worker_group, log_ts, file_path, ok_lines, err_lines, json_fmt) VALUES ($1, $2::text::LOG_MODE, $3, $4, $5, $6, $7, $8)",
if let Err(e) = sqlx::query!("INSERT INTO log_file (hostname, mode, worker_group, log_ts, file_path, ok_lines, err_lines, json_fmt)
VALUES ($1, $2::text::LOG_MODE, $3, $4, $5, $6, $7, $8)
ON CONFLICT (hostname, log_ts) DO UPDATE SET ok_lines = log_file.ok_lines + $6, err_lines = log_file.err_lines + $7",
hostname, mode.to_string(), worker_group.clone(), ts, highest_file, ok_lines as i64, err_lines as i64, *JSON_FMT)
.execute(db)
.await {
+9 -2
View File
@@ -650,8 +650,13 @@ async fn create_script_internal<'c>(
) {
Some(String::new())
} else {
ns.lock
.and_then(|e| if e.is_empty() { None } else { Some(e) })
ns.lock.as_ref().and_then(|e| {
if e.is_empty() {
None
} else {
Some(e.to_string())
}
})
};
let needs_lock_gen = lock.is_none() && codebase.is_none();
@@ -901,6 +906,7 @@ async fn create_script_internal<'c>(
let permissioned_as2 = permissioned_as.clone();
let script_path2 = script_path.clone();
let parent_path = p_path_opt.clone();
let lock = ns.lock.clone();
let deployment_message = ns.deployment_message.clone();
let content = ns.content.clone();
let language = ns.language.clone();
@@ -920,6 +926,7 @@ async fn create_script_internal<'c>(
&authed2.email,
&authed2.username,
&permissioned_as2,
lock,
)
.await
{
+2
View File
@@ -33,6 +33,8 @@ impl<B> OnResponse<B> for MyOnResponse {
let status = response.status().as_u16();
if response.status().is_success() || response.status().is_redirection() {
tracing::info!(latency = latency, status = status, "response")
} else if response.status().as_u16() == 404 {
tracing::warn!(latency = latency, status = status, "response")
} else {
tracing::error!(latency = latency, status = status, "response")
}
+15 -4
View File
@@ -42,7 +42,7 @@ use windmill_common::{
error::{self, Result},
get_latest_hash_for_path,
scripts::ScriptLang,
worker::{exists_in_cache, save_cache, write_file, Connection, DISABLE_BUNDLING},
worker::{exists_in_cache, save_cache, to_raw_value, write_file, Connection, DISABLE_BUNDLING},
DB,
};
@@ -111,7 +111,7 @@ pub async fn gen_bun_lockfile(
let mut empty_deps = false;
if let Some(raw_deps) = raw_deps {
if let Some(raw_deps) = raw_deps.as_ref() {
gen_bunfig(job_dir).await?;
write_file(job_dir, "package.json", raw_deps.as_str())?;
} else {
@@ -201,10 +201,21 @@ pub async fn gen_bun_lockfile(
}
if export_pkg {
let mut content = "".to_string();
let mut content;
{
let mut file = File::open(format!("{job_dir}/package.json")).await?;
file.read_to_string(&mut content).await?;
let mut buf = String::default();
file.read_to_string(&mut buf).await?;
if raw_deps.is_some() {
let mut json_map: HashMap<String, Box<RawValue>> = serde_json::from_str(&buf)?;
json_map.insert(
"generatedFromPackageJson".to_string(),
to_raw_value(&"true".to_string()),
);
content = serde_json::to_string_pretty(&json_map)?;
} else {
content = buf;
}
}
if !npm_mode {
#[cfg(any(target_os = "linux", target_os = "macos"))]
+10 -1
View File
@@ -77,6 +77,7 @@ use crate::{
start_child_process, OccupancyMetrics,
},
handle_child::handle_child,
worker_lockfiles::LOCKFILE_GENERATED_FROM_REQUIREMENTS_TXT,
worker_utils::ping_job_status,
AuthedClient, DISABLE_NSJAIL, DISABLE_NUSER, HOME_ENV, INSTANCE_PYTHON_VERSION, NSJAIL_PATH,
PATH_ENV, PIP_EXTRA_INDEX_URL, PIP_INDEX_URL, PROXY_ENVS, PY_INSTALL_DIR, TZ_ENV, UV_CACHE_DIR,
@@ -2303,8 +2304,16 @@ fn split_requirements(requirements: &str) -> Vec<&str> {
/// Check requirements/lockfile to figure out python version assigned to it.
fn get_pyv_from_requirements_lines(requirements_lines: &[&str]) -> PyVersion {
// If script is deployed we can try to parse first line to get assigned version
let index = if requirements_lines.get(0).map_or(false, |line| {
line.starts_with(LOCKFILE_GENERATED_FROM_REQUIREMENTS_TXT)
}) {
1
} else {
0
};
if let Some(v) = requirements_lines
.get(0)
.get(index)
.and_then(|line| PyVersion::parse_version(*line))
{
// We have valid assigned version, we use it
+51 -16
View File
@@ -65,17 +65,17 @@ pub async fn update_script_dependency_map(
relative_imports: Vec<String>,
) -> error::Result<()> {
let importer_kind = "script";
let mut tx = db.begin().await?;
tx = clear_dependency_parent_path(parent_path, script_path, w_id, importer_kind, tx).await?;
tx = clear_dependency_map_for_item(script_path, w_id, importer_kind, tx, &None).await?;
if !relative_imports.is_empty() {
let mut logs = "".to_string();
logs.push_str("\n--- RELATIVE IMPORTS ---\n\n");
logs.push_str(&relative_imports.join("\n"));
let mut tx = db.begin().await?;
tx =
clear_dependency_parent_path(parent_path, script_path, w_id, importer_kind, tx).await?;
tx = clear_dependency_map_for_item(script_path, w_id, importer_kind, tx, &None).await?;
tx = add_relative_imports_to_dependency_map(
script_path,
w_id,
@@ -86,9 +86,10 @@ pub async fn update_script_dependency_map(
None,
)
.await?;
tx.commit().await?;
append_logs(job_id, w_id, logs, &db.into()).await;
}
tx.commit().await?;
Ok(())
}
@@ -381,6 +382,7 @@ pub async fn handle_dependency_job(
&job.permissioned_as_email,
&job.created_by,
&job.permissioned_as,
None,
)
.await?;
@@ -433,18 +435,42 @@ pub async fn process_relative_imports(
permissioned_as_email: &str,
created_by: &str,
permissioned_as: &str,
lock: Option<String>,
) -> error::Result<()> {
let relative_imports = extract_relative_imports(&code, script_path, script_lang);
if let Some(relative_imports) = relative_imports {
update_script_dependency_map(
&job_id.unwrap_or_else(|| Uuid::nil()),
db,
w_id,
&parent_path,
script_path,
relative_imports,
)
.await?;
if (script_lang.is_some_and(|v| v == ScriptLang::Bun)
&& lock
.as_ref()
.is_some_and(|v| v.contains("generatedFromPackageJson")))
|| (script_lang.is_some_and(|v| v == ScriptLang::Python3)
&& lock
.as_ref()
.is_some_and(|v| v.starts_with(LOCKFILE_GENERATED_FROM_REQUIREMENTS_TXT)))
{
// if the lock file is generated from a package.json/requirements.txt, we need to clear the dependency map
// because we do not want to have dependencies be recomputed automatically. Empty relative imports passed
// to update_script_dependency_map will clear the dependency map.
update_script_dependency_map(
&job_id.unwrap_or_else(|| Uuid::nil()),
db,
w_id,
&parent_path,
script_path,
vec![],
)
.await?;
} else {
update_script_dependency_map(
&job_id.unwrap_or_else(|| Uuid::nil()),
db,
w_id,
&parent_path,
script_path,
relative_imports,
)
.await?;
}
let already_visited = args
.map(|x| {
x.get("already_visited")
@@ -2050,6 +2076,8 @@ async fn ansible_dep(
serde_json::to_string(&ansible_lockfile).map_err(|e| e.into())
}
pub const LOCKFILE_GENERATED_FROM_REQUIREMENTS_TXT: &str = "# from requirements.txt";
async fn capture_dependency_job(
job_id: &Uuid,
job_language: &ScriptLang,
@@ -2118,6 +2146,13 @@ async fn capture_dependency_job(
anns,
)
.await
.map(|res| {
if raw_deps {
format!("{}\n{}", LOCKFILE_GENERATED_FROM_REQUIREMENTS_TXT, res)
} else {
res
}
})
}
}
ScriptLang::Ansible => {