From 0b6d017fedc31e790a76cf29a1adaaf2a72acc61 Mon Sep 17 00:00:00 2001 From: pyranota <92104930+pyranota@users.noreply.github.com> Date: Fri, 11 Apr 2025 23:31:51 +0200 Subject: [PATCH] feat(python): per import requirement pin (#5520) * implement single line pin * make panic-safe * use pin even if multiple modules imported withing single statement * add repins and make imports respect pins * keep all pins * Allow multiple pins * add comments + handle stuff more safely * fix fully qualified imports * remove ignore * sort nested * apply unique to output requirements list * fix typo * remove mut * update sqlx * sort imports * sort imports * fix formatter and format * refactor * fix comptime error * write tests * perf: do not capture if string is empty --- ...43c98104529879f991c49585cd181e34aa827.json | 23 + ...9e631efb65c3d326b6b6ae8361a2116bff145.json | 23 - .../windmill-parser-py-imports/src/lib.rs | 412 ++++++++++++++---- .../windmill-parser-py-imports/tests/tests.rs | 6 +- backend/tests/fixtures/lockfile_python.sql | 51 +++ backend/tests/worker.rs | 203 +++++++++ .../windmill-worker/src/python_executor.rs | 36 +- .../windmill-worker/src/worker_lockfiles.rs | 1 + 8 files changed, 638 insertions(+), 117 deletions(-) create mode 100644 backend/.sqlx/query-443bd83bcea1d37c79cb080095343c98104529879f991c49585cd181e34aa827.json delete mode 100644 backend/.sqlx/query-ac01e45d3335015f53f3d63fe159e631efb65c3d326b6b6ae8361a2116bff145.json create mode 100644 backend/tests/fixtures/lockfile_python.sql diff --git a/backend/.sqlx/query-443bd83bcea1d37c79cb080095343c98104529879f991c49585cd181e34aa827.json b/backend/.sqlx/query-443bd83bcea1d37c79cb080095343c98104529879f991c49585cd181e34aa827.json new file mode 100644 index 0000000000..65b21050c0 --- /dev/null +++ b/backend/.sqlx/query-443bd83bcea1d37c79cb080095343c98104529879f991c49585cd181e34aa827.json @@ -0,0 +1,23 @@ +{ + "db_name": "PostgreSQL", + "query": "\n SELECT content FROM script WHERE path = $1 AND workspace_id = $2\n AND created_at = (SELECT max(created_at) FROM script WHERE path = $1 AND\n workspace_id = $2)\n ", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "content", + "type_info": "Text" + } + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + }, + "nullable": [ + false + ] + }, + "hash": "443bd83bcea1d37c79cb080095343c98104529879f991c49585cd181e34aa827" +} diff --git a/backend/.sqlx/query-ac01e45d3335015f53f3d63fe159e631efb65c3d326b6b6ae8361a2116bff145.json b/backend/.sqlx/query-ac01e45d3335015f53f3d63fe159e631efb65c3d326b6b6ae8361a2116bff145.json deleted file mode 100644 index 929157b5d7..0000000000 --- a/backend/.sqlx/query-ac01e45d3335015f53f3d63fe159e631efb65c3d326b6b6ae8361a2116bff145.json +++ /dev/null @@ -1,23 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "\n SELECT content FROM script WHERE path = $1 AND workspace_id = $2\n AND created_at = (SELECT max(created_at) FROM script WHERE path = $1 AND\n workspace_id = $2)\n ", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "content", - "type_info": "Text" - } - ], - "parameters": { - "Left": [ - "Text", - "Text" - ] - }, - "nullable": [ - false - ] - }, - "hash": "ac01e45d3335015f53f3d63fe159e631efb65c3d326b6b6ae8361a2116bff145" -} diff --git a/backend/parsers/windmill-parser-py-imports/src/lib.rs b/backend/parsers/windmill-parser-py-imports/src/lib.rs index f0c8a4c41a..2f18b74d6b 100644 --- a/backend/parsers/windmill-parser-py-imports/src/lib.rs +++ b/backend/parsers/windmill-parser-py-imports/src/lib.rs @@ -11,6 +11,7 @@ mod mapping; use async_recursion::async_recursion; use itertools::Itertools; use lazy_static::lazy_static; +use std::collections::HashMap; use mapping::{FULL_IMPORTS_MAP, SHORT_IMPORTS_MAP}; #[cfg(not(target_arch = "wasm32"))] @@ -20,6 +21,7 @@ use regex_lite::Regex; use rustpython_parser::{ ast::{Stmt, StmtImport, StmtImportFrom, Suite}, + text_size::TextRange, Parse, }; use sqlx::{Pool, Postgres}; @@ -41,9 +43,10 @@ fn replace_full_import(x: &str) -> Option { lazy_static! { static ref RE: Regex = Regex::new(r"^\#\s?(\S+)\s*$").unwrap(); + static ref PIN_RE: Regex = Regex::new(r"(?:\s*#\s*(pin|repin):\s*)(\S*)").unwrap(); } -fn process_import(module: Option, path: &str, level: usize) -> Vec { +fn process_import(module: Option, path: &str, level: usize) -> Vec { if level > 0 { let mut imports = vec![]; let splitted_path = path.split("/"); @@ -52,17 +55,18 @@ fn process_import(module: Option, path: &str, level: usize) -> Vec error::Result Some(path), + _ => None, }) .collect()); } -fn parse_code_for_imports(code: &str, path: &str) -> error::Result> { +#[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] +enum NImport { + // Order matters! First we want to resolve all repins + + // manually repinned requirement + // e.g.: + // import pandas # repin: pandas==x.y.z + Repin { + pin: ImportPin, + key: String, + }, + // manually pinned requirements + // e.g.: + // import pandas # pin: pandas>=x.y.z + // import pandas # pin: pandas<=x.y.z + // + // NOTE: It is possible for multiple pins exist on same import + // That's why we store vector of pins + Pin { + pins: Vec, + key: String, + }, + // Automatically inferred requirement + // e.g.: + // import pandas + Auto { + // Take `x.y.z` for example + // x is going to be the `root` + // and x.y.z is `full` + // + // `full` will be None if it is equal to root + // + // We will use `root` as a requirement name and pass to `uv pip compile` if it was not replaced with any pin + pkg: String, + + // However we still need full, since all pins pin against full import names + key: Option, + }, + // Relative imports + Relative(String), +} +#[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] +enum NImportResolved { + Repin { pin: ImportPin, key: String }, + Pin { pins: Vec, key: String }, + Auto { pkg: String, key: Option }, +} + +#[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] +struct ImportPin { + pkg: String, + path: String, +} + +fn parse_code_for_imports(code: &str, path: &str) -> error::Result> { let mut code = code.split(DEF_MAIN).next().unwrap_or("").to_string(); // remove main function decorator from end of file if it exists @@ -104,19 +159,56 @@ fn parse_code_for_imports(code: &str, path: &str) -> error::Result> let ast = Suite::parse(&code, "main.py").map_err(|e| { error::Error::ExecutionErr(format!("Error parsing code for imports: {}", e.to_string())) })?; - let nimports: Vec = ast + + let find_pin = |range: TextRange, key: String| { + let hs = code + .chars() + .skip(range.end().to_usize()) + .take_while(|e| *e != '\n') + .collect::(); + + if hs.trim_start().is_empty(){ + return None; + } + + PIN_RE + .captures(&hs) + .and_then(|x| { + x.get(1).zip(x.get(2)).and_then(|(ty_m, pkg_m)| { + let pkg = pkg_m.as_str().to_owned(); + if ty_m.as_str() == "pin" { + Some(vec![NImport::Pin { + pins: vec![ImportPin { pkg, path: path.to_owned() }], + key, + }]) + } else if ty_m.as_str() == "repin" { + Some(vec![NImport::Repin { + pin: ImportPin { pkg, path: path.to_owned() }, + key, + }]) + } else { + None + } + }) + }) + }; + + let mut nimports: Vec = ast .into_iter() .filter_map(|x| match x { - Stmt::Import(StmtImport { names, .. }) => Some( - names - .into_iter() - .map(|x| { - let name = x.name.to_string(); - process_import(Some(name), path, 0) - }) - .flatten() - .collect::>(), - ), + Stmt::Import(StmtImport { names, range }) => names + .get(0) + .and_then(|al| find_pin(range, al.name.to_string())) + .or(Some( + names + .into_iter() + .map(|x| { + let name = x.name.to_string(); + process_import(Some(name), path, 0) + }) + .flatten() + .collect::>(), + )), Stmt::ImportFrom(StmtImportFrom { level: Some(i), module, .. }) if i.to_u32() > 0 => { Some(process_import( module.map(|x| x.to_string()), @@ -124,15 +216,25 @@ fn parse_code_for_imports(code: &str, path: &str) -> error::Result> i.to_usize(), )) } - Stmt::ImportFrom(StmtImportFrom { level: _, module, .. }) => { - Some(process_import(module.map(|x| x.to_string()), path, 0)) - } + Stmt::ImportFrom(StmtImportFrom { level: _, module, range, .. }) => find_pin( + range, + module.clone().map(|x| x.to_string()).unwrap_or_default(), + ) + .or(Some(process_import(module.map(|x| x.to_string()), path, 0))), _ => None, }) .flatten() - .filter(|x| !STDIMPORTS.contains(&x.as_str())) + .filter(|x| { + if let NImport::Auto { ref pkg, .. } = x { + !STDIMPORTS.contains(&(*pkg).as_str()) + } else { + true + } + }) .unique() .collect(); + + nimports.sort(); return Ok(nimports); } @@ -143,8 +245,9 @@ pub async fn parse_python_imports( db: &Pool, already_visited: &mut Vec, annotated_pyv_numeric: &mut Option, -) -> error::Result> { - parse_python_imports_inner( +) -> error::Result<(Vec, Option)> { + let mut compile_error_hint: Option = None; + let mut imports = parse_python_imports_inner( code, w_id, path, @@ -153,7 +256,32 @@ pub async fn parse_python_imports( annotated_pyv_numeric, &mut annotated_pyv_numeric.and_then(|_| Some(path.to_owned())), ) - .await + .await? + .into_values() + .map(|nimport| match nimport { + NImportResolved::Pin { pins, .. } => pins.into_iter().map(|p| { + if let Some(hint) = &mut compile_error_hint{ + hint.push_str(&format!("\n - pin to {} in {}", p.pkg, p.path)); + } else { + compile_error_hint = Some("\n\nMultiple pins can cause problems during lockfile resolution.\nMake sure you checked every pin for conflicts:\n".into()) + }; + Ok(p.pkg) + }).collect_vec(), + NImportResolved::Repin { pin: ImportPin { pkg, .. }, .. } => vec![Ok(pkg)], + NImportResolved::Auto { pkg, ..} => vec![Ok(pkg)], + }) + .flatten() + .collect::>>()? + .into_iter() + .unique() + .collect_vec(); + + imports.sort(); + + compile_error_hint + .as_mut() + .map(|e| e.push_str("\n\nNOTE: You can also `repin` to override all pins")); + Ok((imports, compile_error_hint)) } #[async_recursion] @@ -165,7 +293,7 @@ async fn parse_python_imports_inner( already_visited: &mut Vec, annotated_pyv_numeric: &mut Option, path_where_annotated_pyv: &mut Option, -) -> error::Result> { +) -> error::Result> { let PythonAnnotations { py310, py311, py312, py313, .. } = PythonAnnotations::parse(&code); // we pass only if there is none or only one annotation @@ -194,7 +322,6 @@ async fn parse_python_imports_inner( } else { *annotated_pyv_numeric = Some(numeric); } - *path_where_annotated_pyv = Some(path.to_owned()); } Ok(()) @@ -209,74 +336,205 @@ async fn parse_python_imports_inner( .lines() .find_position(|x| x.starts_with("#requirements:") || x.starts_with("# requirements:")); if let Some((pos, _)) = find_requirements { - let lines = code - .lines() + let mut requirements = HashMap::new(); + code.lines() .skip(pos + 1) .map_while(|x| { - RE.captures(x) - .map(|x| x.get(1).unwrap().as_str().to_string()) + RE.captures(x).and_then(|x| { + x.get(1).map(|m| { + let requirement = m.as_str().to_string(); + requirements.insert( + requirement.clone(), + NImportResolved::Repin { + pin: ImportPin { pkg: requirement, path: Default::default() }, + key: Default::default(), + }, + ); + }) + }) }) - .collect(); - Ok(lines) + .collect_vec(); + + Ok(requirements) } else { let find_extra_requirements = code.lines().find_position(|x| { x.starts_with("#extra_requirements:") || x.starts_with("# extra_requirements:") }); - let mut imports: Vec = vec![]; + let mut imports: HashMap = HashMap::new(); if let Some((pos, _)) = find_extra_requirements { - let lines: Vec = code - .lines() + code.lines() .skip(pos + 1) .map_while(|x| { - RE.captures(x) - .map(|x| x.get(1).unwrap().as_str().to_string()) + RE.captures(x).and_then(|x| { + x.get(1).map(|m| { + let requirement = m.as_str().to_string(); + imports.insert( + requirement.clone(), + NImportResolved::Auto { key: None, pkg: requirement }, + ); + }) + }) }) - .collect(); - imports.extend(lines); + .collect_vec(); } - let nimports = parse_code_for_imports(code, path)?; - for n in nimports.iter() { - let nested = if n.starts_with("relative:") { - let rpath = n.replace("relative:", ""); - let code = sqlx::query_scalar!( - r#" - SELECT content FROM script WHERE path = $1 AND workspace_id = $2 - AND created_at = (SELECT max(created_at) FROM script WHERE path = $1 AND - workspace_id = $2) - "#, - &rpath, - w_id - ) - .fetch_optional(db) - .await? - .unwrap_or_else(|| "".to_string()); + // Will get unsorted vector of imports found in current script + let mut nimports = parse_code_for_imports(code, path)?; - if already_visited.contains(&rpath) { - vec![] - } else { - already_visited.push(rpath.clone()); - parse_python_imports_inner( - &code, - w_id, + // It is important to note, that sorting is important and will always result in this pattern: + // 1. All Repins go first + // 2. All Pins go second + // 3. All Auto go third + // 4. All relative imports go the last + // + // This way we make sure all repins are resolved before (re)pins inside imported relative scripts. + nimports.sort(); + + for n in nimports.into_iter() { + let mut nested = match n { + NImport::Relative(rpath) => { + let code = sqlx::query_scalar!( + r#" + SELECT content FROM script WHERE path = $1 AND workspace_id = $2 + AND created_at = (SELECT max(created_at) FROM script WHERE path = $1 AND + workspace_id = $2) + "#, &rpath, - db, - already_visited, - annotated_pyv_numeric, - path_where_annotated_pyv, + w_id ) + .fetch_optional(db) .await? + .unwrap_or_else(|| "".to_string()); + + if already_visited.contains(&rpath) { + vec![] + } else { + already_visited.push(rpath.clone()); + // Because the algo goes depth first, this function will never return relative import + // This why we can safely assume later, that there is no relative imports + parse_python_imports_inner( + &code, + w_id, + &rpath, + db, + already_visited, + annotated_pyv_numeric, + path_where_annotated_pyv, + ) + .await? + .into_values() + .collect_vec() + } } - } else { - vec![n.to_string()] + NImport::Repin { pin, key } => vec![NImportResolved::Repin { pin, key }], + NImport::Pin { pins, key } => vec![NImportResolved::Pin { pins, key }], + NImport::Auto { pkg, key } => vec![NImportResolved::Auto { pkg, key }], }; + + // Nested should also be sorted for the same reason + nested.sort(); + + // At this point there should be no NImport::Relative in `nested` for imp in nested { - if !imports.contains(&imp) { - imports.push(imp); + let key = match imp.clone() { + NImportResolved::Pin { key, .. } => key, + NImportResolved::Repin { key, .. } => key, + NImportResolved::Auto { key, pkg } => key.unwrap_or(pkg), + }; + // Handled cases: + // + // 1. + // Error: Imported windmill scripts have different pins + // + // auto + // ├── pin:2 + // └── pin:1 + // + // Fix 1: + // + // auto + // ├── pin:1 + // └── pin:1 + // + // Fix 2: + // + // repin:1 + // ├── pin:2 + // └── pin:1 + // + // 2. + // Error: Imported windmill scripts have different pins + // + // pin:2 + // └── pin:1 + // + // Fix 1: + // + // auto + // └── pin:1 + // + // Fix 2: + // + // repin:2 + // └── pin:1 + // + // 3. repins allowed to be repinned again + // + // repin:2 + // └── repin:1 + // + match imp.clone() { + NImportResolved::Repin { .. } => { + if let Some(existing_import) = imports.get(&key) { + match existing_import { + // replace + p if matches!( + p, + NImportResolved::Pin { .. } | NImportResolved::Auto { .. } + ) => + { + imports.insert(key, imp); + } + // do nothing (older repins have greater precedence) + NImportResolved::Repin { .. } => {} + // Should not be possible + _ => { + return Err(anyhow::anyhow!( + "Internal error: cannot resolve requirement pins", + ) + .into()); + } + } + } else { + imports.insert(key, imp.clone()); + } + } + NImportResolved::Pin { pins: new_pins, .. } => { + if let Some(existing_import) = imports.get_mut(&key) { + match existing_import { + // Check if pin is the same version, if same, do nothing, if not error + NImportResolved::Pin { pins: existing_pins, .. } => { + existing_pins.extend(new_pins) + } + // do nothing + NImportResolved::Repin { .. } => {} + // Replace with new pin + NImportResolved::Auto { .. } => { + imports.insert(key, imp); + } + } + } else { + imports.insert(key, imp.clone()); + } + } + NImportResolved::Auto { .. } => { + if !imports.contains_key(&key) { + imports.insert(key, imp); + } + } } } } - imports.sort(); Ok(imports) } } diff --git a/backend/parsers/windmill-parser-py-imports/tests/tests.rs b/backend/parsers/windmill-parser-py-imports/tests/tests.rs index a634f247dd..d734cb8ead 100644 --- a/backend/parsers/windmill-parser-py-imports/tests/tests.rs +++ b/backend/parsers/windmill-parser-py-imports/tests/tests.rs @@ -19,7 +19,7 @@ def main(): "; let mut already_visited = vec![]; - let r = parse_python_imports( + let (r, ..) = parse_python_imports( code, "test-workspace", "f/foo/bar", @@ -52,7 +52,7 @@ def main(): "; let mut already_visited = vec![]; - let r = parse_python_imports( + let (r, ..) = parse_python_imports( code, "test-workspace", "f/foo/bar", @@ -83,7 +83,7 @@ def main(): "; let mut already_visited = vec![]; - let r = parse_python_imports( + let (r, ..) = parse_python_imports( code, "test-workspace", "f/foo/bar", diff --git a/backend/tests/fixtures/lockfile_python.sql b/backend/tests/fixtures/lockfile_python.sql new file mode 100644 index 0000000000..27f7b103f0 --- /dev/null +++ b/backend/tests/fixtures/lockfile_python.sql @@ -0,0 +1,51 @@ +INSERT INTO public.script(workspace_id, created_by, content, schema, summary, description, path, hash, language, lock) VALUES ( +'test-workspace', +'test-user', +' +# requirements: +# microdot==2.2.0 + +import pandas +import requests +import tiny # pin: tiny==0.1.2 + +def main(): + pass +', +'{"$schema":"https://json-schema.org/draft/2020-12/schema","properties":{},"required":[],"type":"object"}', +'', +'', +'f/system/requirements', 12346, 'python3', ''); + +INSERT INTO public.script(workspace_id, created_by, content, schema, summary, description, path, hash, language, lock) VALUES ( +'test-workspace', +'test-user', +' +# extra_requirements: +# bottle==0.13.2 + +import tiny + +def main(): + pass +', +'{"$schema":"https://json-schema.org/draft/2020-12/schema","properties":{},"required":[],"type":"object"}', +'', +'', +'f/system/extra_requirements', 12347, 'python3', ''); + + +INSERT INTO public.script(workspace_id, created_by, content, schema, summary, description, path, hash, language, lock) VALUES ( +'test-workspace', +'test-user', +' +import tiny # pin: bottle==0.13.2 +import simplejson # pin: simplejson==3.19.3 + +def main(): + return [test1(), test2(), test3(), test4()] +', +'{"$schema":"https://json-schema.org/draft/2020-12/schema","properties":{},"required":[],"type":"object"}', +'', +'', +'f/system/pins', 12348, 'python3', ''); diff --git a/backend/tests/worker.rs b/backend/tests/worker.rs index c93256a450..182ed60f2e 100644 --- a/backend/tests/worker.rs +++ b/backend/tests/worker.rs @@ -3831,6 +3831,209 @@ def main(): run_preview_relative_imports(&db, content, ScriptLang::Python3).await; } +async fn assert_lockfile( + db: &Pool, + script_content: String, + language: ScriptLang, + expected_lines: Vec<&str>, +) { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await; + let port = server.addr.port(); + let client = windmill_api_client::create_client( + &format!("http://localhost:{port}"), + "SECRET_TOKEN".to_string(), + ); + + client + .create_script( + "test-workspace", + &NewScript { + language: NewScriptLanguage::from_str(language.as_str()).unwrap(), + content: script_content, + path: "f/system/test_import".to_string(), + concurrent_limit: None, + concurrency_time_window_s: None, + cache_ttl: None, + dedicated_worker: None, + description: "".to_string(), + draft_only: None, + envs: vec![], + is_template: None, + kind: None, + parent_hash: None, + lock: None, + summary: "".to_string(), + tag: None, + schema: std::collections::HashMap::new(), + ws_error_handler_muted: Some(false), + priority: None, + delete_after_use: None, + timeout: None, + restart_unless_cancelled: None, + deployment_message: None, + concurrency_key: None, + visible_to_runner_only: None, + no_main_func: None, + codebase: None, + has_preprocessor: None, + on_behalf_of_email: None, + }, + ) + .await + .unwrap(); + + let mut completed = listen_for_completed_jobs(&db).await; + let db2 = db.clone(); + in_test_worker( + &db, + async move { + completed.next().await; // deployed script + + let script = sqlx::query!( + "SELECT hash FROM script WHERE path = $1", + "f/system/test_import".to_string() + ) + .fetch_one(&db2) + .await + .unwrap(); + + let job = RunJob::from(JobPayload::Dependencies { + path: "f/system/test_import".to_string(), + hash: ScriptHash(script.hash), + dedicated_worker: None, + language, + }) + .push(&db2) + .await; + + completed.next().await; // completed job + + let result = completed_job(job, &db2).await.json_result().unwrap(); + + assert_eq!( + result, + json!({ + "lock": expected_lines.join("\n"), + "status": "Successful lock file generation" + }) + ); + }, + port, + ) + .await; +} +#[sqlx::test(fixtures("base", "lockfile_python"))] +async fn test_requirements_python(db: Pool) { + let content = r#" +# py311 +# requirements: +# tiny==0.1.3 + +import bar +import baz # pin: foo +import baz # repin: fee +import bug # repin: free + +def main(): + pass +"# + .to_string(); + + assert_lockfile( + &db, + content, + ScriptLang::Python3, + vec!["# py311", "tiny==0.1.3"], + ) + .await; +} +#[sqlx::test(fixtures("base", "lockfile_python"))] +async fn test_extra_requirements_python(db: Pool) { + { + let content = r#" +# py311 +# extra_requirements: +# tiny + +import f.system.extra_requirements +import tiny # pin: tiny==0.1.0 +import tiny # pin: tiny==0.1.1 +import tiny # repin: tiny==0.1.2 + +def main(): + pass + "# + .to_string(); + + assert_lockfile( + &db, + content, + ScriptLang::Python3, + vec!["# py311", "bottle==0.13.2", "tiny==0.1.2"], + ) + .await; + } +} +#[sqlx::test(fixtures("base", "lockfile_python"))] +async fn test_extra_requirements_python2(db: Pool) { + + let content = r#" +# py311 +# extra_requirements: +# tiny==0.1.3 + +import simplejson # pin: simplejson==3.20.1 +def main(): + pass +"# + .to_string(); + + assert_lockfile( + &db, + content, + ScriptLang::Python3, + vec![ + "# py311", + "simplejson==3.20.1", + "tiny==0.1.3" + ], + ) + .await; + +} + +#[sqlx::test(fixtures("base", "lockfile_python"))] +async fn test_pins_python(db: Pool) { + let content = r#" +# py311 +# extra_requirements: +# tiny==0.1.3 + +import f.system.requirements +import f.system.pins +import tiny # repin: bottle==0.13.0 +import simplejson + +def main(): + pass +"# + .to_string(); + + assert_lockfile( + &db, + content, + ScriptLang::Python3, + vec![ + "# py311", + "bottle==0.13.0", + "microdot==2.2.0", + "simplejson==3.19.3", + "tiny==0.1.3" + ], + ) + .await; +} #[sqlx::test(fixtures("base", "result_format"))] async fn test_result_format(db: Pool) { let ordered_result_job_id = "1eecb96a-c8b0-4a3d-b1b6-087878c55e41"; diff --git a/backend/windmill-worker/src/python_executor.rs b/backend/windmill-worker/src/python_executor.rs index f07b9abfe2..c642abae49 100644 --- a/backend/windmill-worker/src/python_executor.rs +++ b/backend/windmill-worker/src/python_executor.rs @@ -1439,6 +1439,7 @@ async fn handle_python_deps( .clone(); let mut requirements; + let compilation_error_hint; let mut annotated_pyv = None; let mut annotated_pyv_numeric = None; let is_deployed = requirements_o.is_some(); @@ -1449,23 +1450,26 @@ async fn handle_python_deps( None => { let mut already_visited = vec![]; - requirements = match conn { - Connection::Sql(db) => windmill_parser_py_imports::parse_python_imports( - inner_content, - w_id, - script_path, - db, - &mut already_visited, - &mut annotated_pyv_numeric, - ) - .await? - .join("\n"), + (requirements, compilation_error_hint) = match conn { + Connection::Sql(db) => { + let (r, h) = windmill_parser_py_imports::parse_python_imports( + inner_content, + w_id, + script_path, + db, + &mut already_visited, + &mut annotated_pyv_numeric, + ) + .await?; + + (r.join("\n"), h) + } Connection::Http(_) => match precomputed_agent_info { Some(PrecomputedAgentInfo::Python { py_version, requirements }) => { annotated_pyv_numeric = py_version; - requirements.clone().unwrap_or_else(|| "".to_string()) + (requirements.clone().unwrap_or_else(|| "".to_string()), None) } - _ => "".to_string(), + _ => ("".to_string(), None), }, }; @@ -1487,7 +1491,11 @@ async fn handle_python_deps( ) .await .map_err(|e| { - Error::ExecutionErr(format!("pip compile failed: {}", e.to_string())) + Error::ExecutionErr(format!( + "pip compile failed: {}{}", + e.to_string(), + compilation_error_hint.unwrap_or_default() + )) })?; } &requirements diff --git a/backend/windmill-worker/src/worker_lockfiles.rs b/backend/windmill-worker/src/worker_lockfiles.rs index 8ba9ee501a..6c56f1acef 100644 --- a/backend/windmill-worker/src/worker_lockfiles.rs +++ b/backend/windmill-worker/src/worker_lockfiles.rs @@ -1853,6 +1853,7 @@ async fn capture_dependency_job( &mut annotated_pyv_numeric, ) .await? + .0 .join("\n") };