From a382ed0b2d060ff9d9ff4a93e09254618ec3c62f Mon Sep 17 00:00:00 2001 From: pyranota <92104930+pyranota@users.noreply.github.com> Date: Wed, 4 Jun 2025 17:15:46 +0200 Subject: [PATCH] fix(python): assign PATCH version to python runtime only when needed (#5866) * fix * clean up * nit * fix integration tests * nit * another nit * remove dublicate test --- backend/tests/worker.rs | 12 +- .../windmill-worker/src/python_versions.rs | 214 +++++++++++++++--- 2 files changed, 191 insertions(+), 35 deletions(-) diff --git a/backend/tests/worker.rs b/backend/tests/worker.rs index c8299796f8..8792b2eb1d 100644 --- a/backend/tests/worker.rs +++ b/backend/tests/worker.rs @@ -3969,8 +3969,7 @@ async fn assert_lockfile( #[cfg(feature = "python")] #[sqlx::test(fixtures("base", "lockfile_python"))] async fn test_requirements_python(db: Pool) { - let content = r#" -# py: 3.11.11 + let content = r#"# py: ==3.11.11 # requirements: # tiny==0.1.3 @@ -3997,8 +3996,7 @@ def main(): #[sqlx::test(fixtures("base", "lockfile_python"))] async fn test_extra_requirements_python(db: Pool) { { - let content = r#" -# py: ==3.11.11 + let content = r#"# py: ==3.11.11 # extra_requirements: # tiny @@ -4025,8 +4023,7 @@ def main(): #[cfg(feature = "python")] #[sqlx::test(fixtures("base", "lockfile_python"))] async fn test_extra_requirements_python2(db: Pool) { - let content = r#" -# py: ==3.11.11 + let content = r#"# py: ==3.11.11 # extra_requirements: # tiny==0.1.3 @@ -4048,8 +4045,7 @@ def main(): #[cfg(feature = "python")] #[sqlx::test(fixtures("base", "lockfile_python"))] async fn test_pins_python(db: Pool) { - let content = r#" -# py: ==3.11.11 + let content = r#"# py: ==3.11.11 # extra_requirements: # tiny==0.1.3 # bottle==0.13.2 diff --git a/backend/windmill-worker/src/python_versions.rs b/backend/windmill-worker/src/python_versions.rs index 26be7aca65..28781df616 100644 --- a/backend/windmill-worker/src/python_versions.rs +++ b/backend/windmill-worker/src/python_versions.rs @@ -153,12 +153,18 @@ impl PyV { let valid = all_versions .clone() .into_iter() - .filter(|v| version_specifiers.iter().all(|vs| vs.contains(&*v))) + .filter(|v| version_specifiers.iter().all(|vs| (vs).contains(&*v))) .collect_vec(); if !valid.is_empty() { + let mut result = valid[0].clone(); + // Is there at least one version specifier that has PATCH digit? + let patch_vs = version_specifiers + .iter() + .any(|vs| vs.version().release().get(2).is_some()); + if select_latest { - return Ok(valid[0].clone()); + return Ok(result.clone()); } // Usually INSTANCE_PYTHON_VERSION @@ -183,9 +189,10 @@ impl PyV { // - We will iterate until find the closest version to target. // - If closest version has the same MINOR version, use it. // - If it differs in MINOR version, take latest PATCH version. - // - let mut result = None; + let [major, minor, ..] = result.release() else { + return Err(Error::InternalErr(format!("Failed to parse \"{}\". Available python versions are supposed to be in SEMVER format (MAJOR.MINOR)", *result))); + }; // This represents newest version with oldest MINOR: // // I Iterable Newest in MINOR @@ -195,12 +202,9 @@ impl PyV { // 4. 3.10.2 -> 3.10.2 // 5. 3.10.1 -> 3.10.2 // 6. 3.10.0 -> 3.10.2 - let mut newest_in_minor = None; - for v in valid.iter() { - if result.is_none() { - result.replace(v); - } + let mut newest_in_minor = (result.clone(), (*major, *minor)); + for v in valid.iter() { if v < &gv { // We will not continue if we start looking into versions older than gravity version. break; @@ -212,37 +216,46 @@ impl PyV { // Since we go top to down we can assume // the first occurence of new minor version contains the latest patch version. - if matches!(newest_in_minor, Some((_, mm)) if mm != (major, minor)) - || newest_in_minor.is_none() - { - newest_in_minor = Some((v.clone(), (major, minor))); + if newest_in_minor.1 != (*major, *minor) { + newest_in_minor = (v.clone(), (*major, *minor)); } if gravity_matcher.contains(v) { // return as soon as gravity matcher has first hit. - return Ok(v.clone()); + // Only in case version specifiers do specify PATCH version OR gravity version specify PATCH + if patch_vs || gv.release().get(2).is_some() { + return Ok(v.clone()); + } else { + let Some(release_numbers) = v.release().get(0..=1) else { + return Err(Error::InternalErr(format!( + "Failed to get release numbers from: \"{}\". ", + **v + ))); + }; + return Ok(PyV(pep440_rs::Version::new(release_numbers))); + } } // If we are still in the loop, it means that we are getting closer to gravity version else { - result = Some(v); + result = v.clone(); } } let [gravity_major, gravity_minor, ..] = gv.release() else { - return Err(Error::internal_err(format!("Cannot get MAJOR or/and MINOR version of python gravity version ({}). Something might be wrong with INSTANCE_PYTHON_VERSION.", &*gv))); + return Err(Error::internal_err(format!("Cannot get MAJOR nor MINOR version of python gravity version ({}). Something might be wrong with INSTANCE_PYTHON_VERSION.", &*gv))); }; - if let Some((v, mm)) = newest_in_minor { - if (gravity_major, gravity_minor) != mm { - return Ok(v); + if (*gravity_major, *gravity_minor) != newest_in_minor.1 { + // Return full version only if there is PATCH versions in version specifiers + if patch_vs { + return Ok(newest_in_minor.0); + } else { + let mm = newest_in_minor.1; + return Ok(PyV(pep440_rs::Version::new([mm.0, mm.1]))); } } - result - .ok_or(Error::internal_err( - "No python candidates found. This is a bug!", - )) - .map(ToOwned::to_owned) + Ok(result) } else { Err(anyhow!( " @@ -762,7 +775,7 @@ mod tests { pyv("0.9.3"), pyv("0.9.2"), ], - pyv("0.9.4"), // + pyv("0.9"), // ) .await; } @@ -785,7 +798,7 @@ mod tests { pyv("0.8.1"), pyv("0.8.0"), ], - pyv("1.0.2"), // + pyv("1.0"), // ) .await; } @@ -824,7 +837,7 @@ mod tests { pyv("2.2.1"), pyv("2.2.0"), ], - pyv("2.2.2"), + pyv("2.2"), ) .await; } @@ -845,4 +858,151 @@ mod tests { ) .await; } + #[tokio::test] + async fn test_python_resolution_8() { + assert_resolution( + "2.2", + false, + vec![">2.2", ">=2.4", "<2.4.1"], + vec![ + pyv("2.4.1"), + pyv("2.4.0"), + pyv("2.3.1"), + pyv("2.3.0"), + pyv("2.2.1"), + pyv("2.2.0"), + pyv("2.1.1"), + pyv("2.1.0"), + ], + pyv("2.4.0"), + ) + .await; + } + #[tokio::test] + async fn test_python_resolution_9() { + assert_resolution( + "2.2", + true, + vec![">2.2", ">2.3", "<2.4.1"], + vec![ + pyv("2.4.1"), + pyv("2.4.0"), + pyv("2.3.1"), + pyv("2.3.0"), + pyv("2.2.1"), + pyv("2.2.0"), + pyv("2.1.1"), + pyv("2.1.0"), + ], + pyv("2.4.0"), + ) + .await; + } + #[tokio::test] + async fn test_python_resolution_10() { + assert_resolution( + "2.2", + false, + vec![">2.2", ">=2.4"], + vec![ + pyv("2.4.1"), + pyv("2.4.0"), + pyv("2.3.1"), + pyv("2.3.0"), + pyv("2.2.1"), + pyv("2.2.0"), + pyv("2.1.1"), + pyv("2.1.0"), + ], + // vec![pyv("2.4.1"), pyv("2.3"), pyv("2.2"), pyv("2.1")], + pyv("2.4"), + ) + .await; + } + #[tokio::test] + async fn test_python_resolution_11() { + assert_resolution( + "2.4.1", + false, + vec![">2.2", ">=2.3", "<2.4"], + vec![ + pyv("2.4.1"), + pyv("2.4.0"), + pyv("2.3.1"), + pyv("2.3.0"), + pyv("2.2.1"), + pyv("2.2.0"), + pyv("2.1.1"), + pyv("2.1.0"), + ], + // vec![pyv("2.4.1"), pyv("2.3"), pyv("2.2"), pyv("2.1")], + pyv("2.3"), + ) + .await; + } + #[tokio::test] + async fn test_python_resolution_12() { + assert_resolution( + "2.3", + false, + vec![">2.2", ">=2.3", "<2.4"], + vec![ + pyv("2.4.1"), + pyv("2.4.0"), + pyv("2.3.1"), + pyv("2.3.0"), + pyv("2.2.1"), + pyv("2.2.0"), + pyv("2.1.1"), + pyv("2.1.0"), + ], + // vec![pyv("2.4.1"), pyv("2.3"), pyv("2.2"), pyv("2.1")], + pyv("2.3"), + ) + .await; + } + #[tokio::test] + async fn test_python_resolution_13() { + assert_resolution( + "2.3.1", + false, + vec![">2.2", ">=2.3", "<2.4"], + vec![ + pyv("2.4.1"), + pyv("2.4.0"), + pyv("2.3.1"), + pyv("2.3.0"), + pyv("2.2.1"), + pyv("2.2.0"), + pyv("2.1.1"), + pyv("2.1.0"), + ], + // vec![pyv("2.4.1"), pyv("2.3"), pyv("2.2"), pyv("2.1")], + pyv("2.3.1"), + ) + .await; + } + #[tokio::test] + async fn test_python_resolution_14() { + assert_resolution( + "2.3.0", + false, + vec![], + vec![pyv("2.4.1"), pyv("2.4.0"), pyv("2.3.1")], + pyv("2.3.1"), + ) + .await; + } + + #[tokio::test] + async fn test_python_resolution_16() { + assert_resolution( + "2.3", + false, + vec![], + vec![pyv("2.4.1"), pyv("2.4.0"), pyv("2.3.1")], + pyv("2.3"), + ) + .await; + } }