Explicitly close the temp file

crates/sparrow-main/src/serve/file_service.rs

@@ -75,10 +75,9 @@ pub(crate) async fn get_source_metadata(
     s3: &S3Helper,
     source_data: &SourceData,
 ) -> anyhow::Result<SourceMetadata> {
-    let download_file = NamedTempFile::new().unwrap();
-    let download_file_path = download_file.into_temp_path();
+    let temp_file = NamedTempFile::new().context("Unable to create temporary file")?;
     let (_, local_source) =
-        preparation_service::convert_to_local_sourcedata(s3, Some(source_data), download_file_path)
+        preparation_service::convert_to_local_sourcedata(s3, Some(source_data), temp_file.path())
             .await
             .map_err(|e| anyhow!("Unable to convert source to local source: {:?}", e))?;
 
@@ -91,6 +90,11 @@ pub(crate) async fn get_source_metadata(
         )
     })?;
 
+    // Explicitly close the temporary file here. This ensures that
+    // the temporary file cleaner on different OS's know to not remove
+    // the file at least until this point.
+    temp_file.close()?;
+
     Ok(SourceMetadata {
         schema: Some(schema),
     })

crates/sparrow-main/src/serve/preparation_service.rs

@@ -80,14 +80,12 @@ pub async fn prepare_data(
         }
     );
 
-    let download_file = NamedTempFile::new().unwrap();
-    let download_file_path = download_file.into_temp_path();
-    let (is_s3_object, source_data) = convert_to_local_sourcedata(
-        &s3,
-        prepare_request.source_data.as_ref(),
-        download_file_path,
-    )
-    .await?;
+    let temp_file = NamedTempFile::new()
+        .into_report()
+        .change_context(Error::Internal)?;
+    let (is_s3_object, source_data) =
+        convert_to_local_sourcedata(&s3, prepare_request.source_data.as_ref(), temp_file.path())
+            .await?;
 
     let temp_dir = tempfile::tempdir()
         .into_report()
@@ -128,7 +126,7 @@ pub async fn prepare_data(
 pub async fn convert_to_local_sourcedata(
     s3: &S3Helper,
     source_data: Option<&SourceData>,
-    download_file_path: tempfile::TempPath,
+    local_path: &Path,
 ) -> error_stack::Result<(bool, SourceData), Error> {
     match source_data {
         None => error_stack::bail!(Error::MissingField("source_data")),
@@ -143,13 +141,11 @@ pub async fn convert_to_local_sourcedata(
                             let path = path.as_str();
                             if is_s3_path(path) {
                                 let s3_object = S3Object::try_from_uri(path).unwrap();
-                                s3.download_s3(s3_object, download_file_path.to_owned())
-                                    .await
-                                    .unwrap();
+                                s3.download_s3(s3_object, local_path).await.unwrap();
                                 (
                                     true,
                                     source_data::Source::ParquetPath(
-                                        download_file_path.to_string_lossy().to_string(),
+                                        local_path.to_string_lossy().to_string(),
                                     ),
                                 )
                             } else {
@@ -160,21 +156,19 @@ pub async fn convert_to_local_sourcedata(
                             let path = path.as_str();
                             if is_s3_path(path) {
                                 let s3_object = S3Object::try_from_uri(path).unwrap();
-                                s3.download_s3(s3_object, download_file_path.to_owned())
-                                    .await
-                                    .unwrap();
+                                s3.download_s3(s3_object, local_path).await.unwrap();
                                 (
                                     true,
                                     source_data::Source::CsvPath(
-                                        download_file_path.to_string_lossy().to_string(),
+                                        local_path.to_string_lossy().to_string(),
                                     ),
                                 )
                             } else {
                                 (false, source_data::Source::CsvPath(path.to_string()))
                             }
                         }
                         source_data::Source::CsvData(data) => {
-                            (false, source_data::Source::CsvData(data.to_string()))
+                            (false, source_data::Source::CsvData(data.to_owned()))
                         }
                         source_data::Source::PulsarSubscription(_) => (false, source.clone()),
                     };