Skip to content

Commit

Permalink
Use specialized error message for invalid Python install / uninstall …
Browse files Browse the repository at this point in the history
…requests (#5171)

## Summary

Closes #4819.
  • Loading branch information
charliermarsh committed Jul 18, 2024
1 parent 622e9e8 commit 91bf213
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 24 deletions.
27 changes: 11 additions & 16 deletions crates/uv-python/src/downloads.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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> {
Self::from_request(request).ok()
}

/// Construct a new [`PythonDownloadRequest`] from a [`PythonRequest`].
pub fn from_request(request: &PythonRequest) -> Result<Self, Error> {
pub fn from_request(request: &PythonRequest) -> Option<Self> {
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,
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/uv-python/src/installation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand All @@ -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),
Expand Down
10 changes: 7 additions & 3 deletions crates/uv/src/commands/python/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -67,8 +67,12 @@ pub(crate) async fn install(

let download_requests = requests
.iter()
.map(PythonDownloadRequest::from_request)
.collect::<Result<Vec<_>, downloads::Error>>()?;
.map(|request| {
PythonDownloadRequest::from_request(request).ok_or_else(|| {
anyhow::anyhow!("Cannot download managed Python for request: {request}")
})
})
.collect::<Result<Vec<_>>>()?;

let installed_installations: Vec<_> = installations.find_all()?.collect();
let mut unfilled_requests = Vec::new();
Expand Down
10 changes: 7 additions & 3 deletions crates/uv/src/commands/python/uninstall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -43,8 +43,12 @@ pub(crate) async fn uninstall(

let download_requests = requests
.iter()
.map(PythonDownloadRequest::from_request)
.collect::<Result<Vec<_>, downloads::Error>>()?;
.map(|request| {
PythonDownloadRequest::from_request(request).ok_or_else(|| {
anyhow::anyhow!("Cannot uninstall managed Python for request: {request}")
})
})
.collect::<Result<Vec<_>>>()?;

let installed_installations: Vec<_> = installations.find_all()?.collect();
let mut matching_installations = BTreeSet::default();
Expand Down

0 comments on commit 91bf213

Please sign in to comment.