-
Notifications
You must be signed in to change notification settings - Fork 750
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
fix parquet_derive with default features (and fix cargo publish
)
#837
Conversation
cargo publish
)
@@ -54,8 +54,8 @@ snap = "1.0" | |||
brotli = "3.3" | |||
flate2 = "1.0" | |||
lz4 = "1.23" | |||
arrow = { path = "../arrow", version = "7.0.0-SNAPSHOT" } |
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 use of arrow with default features as a dev
dependency masked the problem with parquet_derive in tests.
@@ -41,7 +41,7 @@ lz4 = { version = "1.23", optional = true } | |||
zstd = { version = "0.9", optional = true } | |||
chrono = "0.4" | |||
num-bigint = "0.4" | |||
arrow = { path = "../arrow", version = "7.0.0-SNAPSHOT", optional = true, default-features = false } | |||
arrow = { path = "../arrow", version = "7.0.0-SNAPSHOT", optional = true, default-features = false, features = ["ipc"] } |
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 the fix of the actual problem: the arrow writer requires the ipc
feature which is no longer a default
@@ -0,0 +1,21 @@ | |||
<!--- |
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 follows the pattern of the tests in arrow/test/dependency
Codecov Report
@@ Coverage Diff @@
## master #837 +/- ##
==========================================
+ Coverage 82.57% 82.60% +0.02%
==========================================
Files 168 168
Lines 47946 48040 +94
==========================================
+ Hits 39593 39685 +92
- Misses 8353 8355 +2
Continue to review full report at Codecov.
|
I don't really understand why tests like https://github.com/apache/arrow-rs/pull/837/checks?check_run_id=3927127187 are failing with errors like
I am trying a workaround with 89fa7c4 (the theory being there is something wrong with the cargo cache that is cached in github). We'll see how that works |
Filed #838 for the CI problem |
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.
Thanks @alamb for the quick fix
89fa7c4
to
7c32795
Compare
…ublish) (#856) * fix parquet_derive with default features (and fix `cargo publish`) (#837) * Run all tests and do dry runs of cargo publish * Add test for building parquet derive with default features' * fix feature flags in parquet crate * fixup rat * fix default feature test * Update parquet_derive/test/dependency/default-features/Cargo.toml * Remove merge issue
Which issue does this PR close?
Resolves #833
Resolved #840
Rationale for this change
cargo publish
for parquet_derive is broken (turns out parquet_derive with default feature flags is broken)What changes are included in this PR?
(thanks @houqp!)
datafusion
's check and runcaro test --all
Are there any user-facing changes?
parquet_derive can be used standalone again