Skip to content

Comments

Drop Python version range enforcement from PythonVersion::from_str#7264

Merged
zanieb merged 1 commit intomainfrom
zb/python-version-enforcement
Sep 10, 2024
Merged

Drop Python version range enforcement from PythonVersion::from_str#7264
zanieb merged 1 commit intomainfrom
zb/python-version-enforcement

Conversation

@zanieb
Copy link
Member

@zanieb zanieb commented Sep 10, 2024

This caused some problems earlier, as it prevented us from listing Python versions <3.7 which seems weird (see #7131 (comment))

I'm worried that without this the changes to installation key parsing in #7263 would otherwise be too restrictive.

I think if we want to enforce these ranges, we should do so separately from the parse step.

@zanieb
Copy link
Member Author

zanieb commented Sep 10, 2024

@konstin looking to add this validation somewhere but not sure where it belongs

It looks like this error variant is never used?

#[error("Python {python_version} is not supported. Please use Python 3.8 or newer.")]
UnsupportedPythonVersion { python_version: String },

We probably need to check this during version requests too?

For future reference, explicit validation looks like this

diff --git a/crates/uv-python/src/python_version.rs b/crates/uv-python/src/python_version.rs
index 6c4a99487..db83bb4ac 100644
--- a/crates/uv-python/src/python_version.rs
+++ b/crates/uv-python/src/python_version.rs
@@ -165,6 +165,18 @@ impl PythonVersion {
         Self::from_str(format!("{}.{}", self.major(), self.minor()).as_str())
             .expect("dropping a patch should always be valid")
     }
+
+    /// Returns an error if the Python version is not supported.
+    pub fn check_supported(&self) -> Result<(), String> {
+        if version.version < Version::new([3, 7]) {
+            return Err(format!("Python version `{s}` must be >= 3.7"));
+        }
+        if version.version >= Version::new([4, 0]) {
+            return Err(format!("Python version `{s}` must be < 4.0"));
+        }
+
+        Ok(())
+    }
 }
 
 #[cfg(test)]

@zanieb zanieb marked this pull request as ready for review September 10, 2024 18:15
@konstin
Copy link
Member

konstin commented Sep 10, 2024

We should be filtering out too old interpreters at

if sys.version_info < (3, 7):

@zanieb
Copy link
Member Author

zanieb commented Sep 10, 2024

Ah okay so it's constructed by the deserializer. Lgtm then.

@zanieb zanieb merged commit 533c7e3 into main Sep 10, 2024
@zanieb zanieb deleted the zb/python-version-enforcement branch September 10, 2024 18:34
zanieb added a commit that referenced this pull request Sep 10, 2024
…7269)

Follows test cases in #7265 and validation removal in
#7264

It turns out we don't have good error messages for these as-is.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants