kosiew commented on code in PR #21504:
URL: https://github.com/apache/datafusion/pull/21504#discussion_r3122489637
##########
Cargo.toml:
##########
@@ -105,6 +105,7 @@ arrow-flight = { version = "58.1.0", features = [
] }
arrow-ipc = { version = "58.1.0", default-features = false, features = [
Review Comment:
Nice fix. Moving `arrow-ipc/zstd` into the workspace dependency clears up
the test coupling cleanly.
Could we add a short comment here explaining that this extra codec is needed
for spill-file support? That would make the tradeoff more obvious and help
prevent a future dependency cleanup from accidentally bringing back the same
failure mode.
##########
datafusion/datasource-arrow/Cargo.toml:
##########
@@ -62,6 +62,6 @@ name = "datafusion_datasource_arrow"
path = "src/mod.rs"
[features]
-compression = [
- "arrow-ipc/zstd",
-]
+# This feature is deprecated, as core functionality in the SpillManager
requires all features
Review Comment:
Thanks for keeping the `compression` feature around for compatibility. Since
it is now effectively a no-op, could we leave a more visible deprecation
breadcrumb for downstream users, such as a changelog note or a brief
crate-level note?
That would make it clearer that IPC compression is now always enabled,
instead of users thinking it is still gated somewhere else.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
