alamb commented on code in PR #7532:
URL: https://github.com/apache/arrow-datafusion/pull/7532#discussion_r1323548729
##########
datafusion/core/src/datasource/file_format/write.rs:
##########
@@ -513,17 +530,20 @@ pub(crate) async fn stateless_serialize_and_write_files(
any_abort_errors = true;
}
}
- false => writer.shutdown().await?,
+ false => writer
+ .shutdown()
+ .await
+ .map_err(|e| DataFusionError::io_error(e,
writer.description()))?,
Review Comment:
This passes along information about to what location the writer was writing
##########
datafusion/core/src/datasource/file_format/write.rs:
##########
@@ -513,17 +530,20 @@ pub(crate) async fn stateless_serialize_and_write_files(
any_abort_errors = true;
}
}
- false => writer.shutdown().await?,
+ false => writer
+ .shutdown()
+ .await
+ .map_err(|e| DataFusionError::io_error(e,
writer.description()))?,
}
}
}
if any_errors {
match any_abort_errors{
- true => return Err(DataFusionError::Internal("Error encountered
during writing to ObjectStore and failed to abort all writers. Partial result
may have been written.".into())),
Review Comment:
drive by cleanup
##########
datafusion/core/src/datasource/physical_plan/json.rs:
##########
@@ -282,14 +283,15 @@ pub async fn plan_to_json(
let mut writer = json::LineDelimitedWriter::new(buffer);
writer.write(&batch)?;
buffer = writer.into_inner();
- multipart_writer.write_all(&buffer).await?;
+ multipart_writer.write_all(&buffer).await.map_err(|e| {
Review Comment:
basically the point of this PR is to annotate callsites like this with
information about where they were writing to
##########
datafusion/core/src/datasource/listing/table.rs:
##########
@@ -1257,9 +1257,9 @@ mod tests {
#[tokio::test]
async fn unbounded_csv_table_without_schema() -> Result<()> {
- let tmp_dir = TempDir::new()?;
Review Comment:
strictly speaking these changes are unecessary -- they are a result of how I
found the locations to add `DataFusion::io_error` which was to comment out the
`From<std::io::Error>` impl for `DataFusionError`
I can revert them if people prefer, but I actually think having errors fail
fast (with `unwrap`) makes them easier to understand and debug when a failure
does occur
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
