Skip to content

Commit f891e5c

Browse files
docs: clarify file deletion behavior on write failure
1 parent ab4de8c commit f891e5c

File tree

1 file changed

+18
-18
lines changed

1 file changed

+18
-18
lines changed

pre-compute/src/compute/utils/file_utils.rs

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ pub fn write_file(content: &[u8], file_path: &Path, context: &str) -> Result<(),
5454
///
5555
/// If the download or any file operation fails, the function logs an appropriate error
5656
/// and returns `None`. It also ensures the parent directory exists, creating it if necessary.
57-
/// If the directory is newly created but the file write fails, it is cleaned up (deleted).
57+
/// If the file write fails and a partial file was created, it will be cleaned up (deleted).
5858
///
5959
/// # Arguments
6060
///
@@ -115,24 +115,24 @@ pub fn download_file(url: &str, parent_dir: &str, filename: &str) -> Option<Path
115115

116116
let file_path = parent_path.join(filename);
117117

118-
if write_file(&bytes, &file_path, &format!("url:{url}")).is_ok() {
119-
Some(file_path)
120-
} else {
121-
if file_path.exists() {
122-
match fs::remove_file(&file_path) {
123-
Ok(_) => {
124-
info!("File deleted [path:{}]", file_path.display());
125-
}
126-
Err(e) => {
127-
error!(
128-
"Failed to delete file [path:{}, error:{}]",
129-
file_path.display(),
130-
e
131-
);
118+
match write_file(&bytes, &file_path, &format!("url:{url}")) {
119+
Ok(_) => Some(file_path),
120+
Err(_) => {
121+
if file_path.exists() {
122+
match fs::remove_file(&file_path) {
123+
Ok(_) => {
124+
info!("File deleted [path:{}]", file_path.display());
125+
}
126+
Err(e) => {
127+
error!(
128+
"Failed to delete file [path:{}, error:{e}]",
129+
file_path.display()
130+
);
131+
}
132132
}
133133
}
134+
None
134135
}
135-
None
136136
}
137137
}
138138

@@ -315,7 +315,7 @@ mod tests {
315315

316316
#[test]
317317
#[cfg(unix)]
318-
fn test_download_fails_on_file_write_error_preserves_parent() {
318+
fn download_from_url_fails_and_preserves_parent_dir_when_file_write_error() {
319319
use std::os::unix::fs::PermissionsExt;
320320
let (_container, container_url) = start_container();
321321

@@ -337,7 +337,7 @@ mod tests {
337337

338338
#[test]
339339
#[cfg(unix)]
340-
fn test_file_deleted_when_write_fails_with_existing_parent() {
340+
fn download_from_url_fails_and_file_deleted_when_write_partialy_fails() {
341341
use std::os::unix::fs::PermissionsExt;
342342
let (_container, container_url) = start_container();
343343

0 commit comments

Comments
 (0)