-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update prost
, prost-derive
, pbjson
, tonic
ecosystem
#11372
Conversation
🤔 looks like maybe we need to update arrow-flight's dependencies too 🤔 |
Co-authored-by: tison <[email protected]>
Thank you @tisonkun It appears to me that compilation is failing now as arrow-flight is using an older version of prost/tonic, but since datafusion itself doesn't depend on any of those, I reverted the upgrade |
pub enum BloomFilterFppOpt { | ||
#[prost(double, tag = "6")] | ||
BloomFilterFpp(f64), | ||
} | ||
#[allow(clippy::derive_partial_eq_without_eq)] | ||
#[derive(Clone, PartialEq, ::prost::Oneof)] | ||
#[derive(Clone, Copy, PartialEq, ::prost::Oneof)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just wondering why the Copy was added?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is generated code. so I think it comes from the changes in prost. I think it comes from tokio-rs/prost#950 specifically
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm thanks @alamb
I believe if we merge this PR, people won't be able to upgrade DataFusion with their systems that use arrow-flight unil this is also released: apache/arrow-rs#6041 |
To be clear, this is blocked on the release of arrow-flight |
Superceded by #12032 |
NOTE: If we merge this PR before arrow-flight is released with an upgraded version of prost, I think DataFusion based systems might not be able to upgrade DataFusion until that new flight version is available
I didn't look carefully, but if I updated prost/tonic in datafusion-examples (needed for the flight server example) I got a bunch of compiler errors
Which issue does this PR close?
Closes #11352
closes #11353
and several other dependency PR updates
Rationale for this change
Keep up to date with dependencies
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?