From f5770fa491c85f9d2f9ca63be840c0ecaa72e7da Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 17 Jul 2024 19:39:20 -0400 Subject: [PATCH] Use specialized error message for invalid Python install / uninstall requests --- crates/uv-python/src/downloads.rs | 27 +++++++++------------- crates/uv-python/src/installation.rs | 4 ++-- crates/uv/src/commands/python/install.rs | 10 +++++--- crates/uv/src/commands/python/uninstall.rs | 10 +++++--- 4 files changed, 27 insertions(+), 24 deletions(-) diff --git a/crates/uv-python/src/downloads.rs b/crates/uv-python/src/downloads.rs index 6bf5c08b058e..0ef50096acfa 100644 --- a/crates/uv-python/src/downloads.rs +++ b/crates/uv-python/src/downloads.rs @@ -68,8 +68,6 @@ pub enum Error { NameError(String), #[error("Failed to parse request part")] InvalidRequestPlatform(#[from] platform::Error), - #[error("Cannot download managed Python for request: {0}")] - InvalidRequestKind(PythonRequest), // TODO(zanieb): Implement display for `PythonDownloadRequest` #[error("No download found for request: {0:?}")] NoDownloadFound(PythonDownloadRequest), @@ -142,26 +140,23 @@ impl PythonDownloadRequest { /// /// Returns [`None`] if the request kind is not compatible with a download, e.g., it is /// a request for a specific directory or executable name. - pub fn try_from_request(request: &PythonRequest) -> Option { - Self::from_request(request).ok() - } - - /// Construct a new [`PythonDownloadRequest`] from a [`PythonRequest`]. - pub fn from_request(request: &PythonRequest) -> Result { + pub fn from_request(request: &PythonRequest) -> Option { match request { - PythonRequest::Version(version) => Ok(Self::default().with_version(version.clone())), + PythonRequest::Version(version) => Some(Self::default().with_version(version.clone())), PythonRequest::Implementation(implementation) => { - Ok(Self::default().with_implementation(*implementation)) + Some(Self::default().with_implementation(*implementation)) } - PythonRequest::ImplementationVersion(implementation, version) => Ok(Self::default() - .with_implementation(*implementation) - .with_version(version.clone())), - PythonRequest::Key(request) => Ok(request.clone()), - PythonRequest::Any => Ok(Self::default()), + PythonRequest::ImplementationVersion(implementation, version) => Some( + Self::default() + .with_implementation(*implementation) + .with_version(version.clone()), + ), + PythonRequest::Key(request) => Some(request.clone()), + PythonRequest::Any => Some(Self::default()), // We can't download a managed installation for these request kinds PythonRequest::Directory(_) | PythonRequest::ExecutableName(_) - | PythonRequest::File(_) => Err(Error::InvalidRequestKind(request.clone())), + | PythonRequest::File(_) => None, } } diff --git a/crates/uv-python/src/installation.rs b/crates/uv-python/src/installation.rs index 11a15ff73bac..75b8c2c17dc5 100644 --- a/crates/uv-python/src/installation.rs +++ b/crates/uv-python/src/installation.rs @@ -90,7 +90,7 @@ impl PythonInstallation { // Perform a fetch aggressively if managed Python is preferred if matches!(preference, PythonPreference::Managed) && python_fetch.is_automatic() { - if let Some(request) = PythonDownloadRequest::try_from_request(&request) { + if let Some(request) = PythonDownloadRequest::from_request(&request) { return Self::fetch(request.fill(), client_builder, cache, reporter).await; } } @@ -104,7 +104,7 @@ impl PythonInstallation { && python_fetch.is_automatic() && client_builder.connectivity.is_online() => { - if let Some(request) = PythonDownloadRequest::try_from_request(&request) { + if let Some(request) = PythonDownloadRequest::from_request(&request) { debug!("Requested Python not found, checking for available download..."); match Self::fetch(request.fill(), client_builder, cache, reporter).await { Ok(installation) => Ok(installation), diff --git a/crates/uv/src/commands/python/install.rs b/crates/uv/src/commands/python/install.rs index eab059549174..1a37129f45fd 100644 --- a/crates/uv/src/commands/python/install.rs +++ b/crates/uv/src/commands/python/install.rs @@ -12,7 +12,7 @@ use uv_cache::Cache; use uv_client::Connectivity; use uv_configuration::PreviewMode; use uv_fs::Simplified; -use uv_python::downloads::{self, DownloadResult, ManagedPythonDownload, PythonDownloadRequest}; +use uv_python::downloads::{DownloadResult, ManagedPythonDownload, PythonDownloadRequest}; use uv_python::managed::{ManagedPythonInstallation, ManagedPythonInstallations}; use uv_python::{ requests_from_version_file, PythonRequest, PYTHON_VERSIONS_FILENAME, PYTHON_VERSION_FILENAME, @@ -67,8 +67,12 @@ pub(crate) async fn install( let download_requests = requests .iter() - .map(PythonDownloadRequest::from_request) - .collect::, downloads::Error>>()?; + .map(|request| { + PythonDownloadRequest::from_request(request).ok_or_else(|| { + anyhow::anyhow!("Cannot download managed Python for request: {request}") + }) + }) + .collect::>>()?; let installed_installations: Vec<_> = installations.find_all()?.collect(); let mut unfilled_requests = Vec::new(); diff --git a/crates/uv/src/commands/python/uninstall.rs b/crates/uv/src/commands/python/uninstall.rs index 79b98293eae6..1353bab1cb53 100644 --- a/crates/uv/src/commands/python/uninstall.rs +++ b/crates/uv/src/commands/python/uninstall.rs @@ -7,7 +7,7 @@ use itertools::Itertools; use owo_colors::OwoColorize; use uv_configuration::PreviewMode; -use uv_python::downloads::{self, PythonDownloadRequest}; +use uv_python::downloads::PythonDownloadRequest; use uv_python::managed::ManagedPythonInstallations; use uv_python::PythonRequest; use uv_warnings::warn_user_once; @@ -43,8 +43,12 @@ pub(crate) async fn uninstall( let download_requests = requests .iter() - .map(PythonDownloadRequest::from_request) - .collect::, downloads::Error>>()?; + .map(|request| { + PythonDownloadRequest::from_request(request).ok_or_else(|| { + anyhow::anyhow!("Cannot uninstall managed Python for request: {request}") + }) + }) + .collect::>>()?; let installed_installations: Vec<_> = installations.find_all()?.collect(); let mut matching_installations = BTreeSet::default();