Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Sep 10, 2024

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
58 checks passed
@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