Skip to content

Commit

Permalink
Remove implicit conversion from std::io::Error to StoreError (#18696
Browse files Browse the repository at this point in the history
)

Remove implicit conversion from `std::io::Error` to `StoreError`, since
(as described on #18587) the former generally does not contain enough
information to debug an issue.

Fixes #18587.
  • Loading branch information
stuhood authored Apr 6, 2023
1 parent 317a09d commit cb01cbe
Showing 1 changed file with 19 additions and 10 deletions.
29 changes: 19 additions & 10 deletions src/rust/engine/fs/store/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ use std::fmt::{self, Debug, Display};
use std::fs::OpenOptions;
use std::fs::Permissions as FSPermissions;
use std::future::Future;
use std::io::{self, Write};
use std::io::Write;
use std::os::unix::fs::{MetadataExt, OpenOptionsExt, PermissionsExt};
use std::path::{Path, PathBuf};
use std::sync::{Arc, Weak};
Expand Down Expand Up @@ -143,12 +143,6 @@ impl From<String> for StoreError {
}
}

impl From<io::Error> for StoreError {
fn from(err: io::Error) -> Self {
Self::Unclassified(err.to_string())
}
}

// Summary of the files and directories uploaded with an operation
// ingested_file_{count, bytes}: Number and combined size of processed files
// uploaded_file_{count, bytes}: Number and combined size of files uploaded to the remote
Expand Down Expand Up @@ -1448,7 +1442,12 @@ impl Store {
destination: PathBuf,
target: String,
) -> Result<(), StoreError> {
symlink(target, destination).await?;
symlink(&target, &destination).await.map_err(|e| {
format!(
"Failed to create symlink to {target} at {}: {e}",
destination.display()
)
})?;
Ok(())
}

Expand All @@ -1464,9 +1463,19 @@ impl Store {
// It also has the benefit of playing nicely with Docker for macOS file virtualization: see
// #18162.
#[cfg(target_os = "macos")]
copy(target, destination).await?;
copy(&target, &destination).await.map_err(|e| {
format!(
"Failed to copy from {target} to {}: {e}",
destination.display()
)
})?;
#[cfg(not(target_os = "macos"))]
hard_link(target, destination).await?;
hard_link(&target, &destination).await.map_err(|e| {
format!(
"Failed to create hardlink to {target} at {}: {e}",
destination.display()
)
})?;
Ok(())
}

Expand Down

0 comments on commit cb01cbe

Please sign in to comment.