You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/07/31 10:45:50 UTC
[GitHub] [arrow] vertexclique opened a new pull request #7873: ARROW-9609 - Leaner feature gating for arrow in parquet
vertexclique opened a new pull request #7873:
URL: https://github.com/apache/arrow/pull/7873
Currently, the parquet is installing arrow-flight and it's dependencies, which breaks the CI builds and it's unnecessary because it is not used. Parquet should work without any default features by default. Simple PR will enable building it leaner.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] vertexclique edited a comment on pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
vertexclique edited a comment on pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#issuecomment-668032973
Parquet crate is using arrow's default features which are `default = ["flight", "prettyprint"]` but flight is not needed, nor prettyprint for parquet. these are not used at all. Moreover, since features are additive, we can't override arrow dependencies features in any project that uses parquet if we continue to use default features included.
One more nit; it seems like we've added the same dependency as a dev dependency. Which is also not needed because it is in the default dependency list.
> ... it seems inverted that arrow depends on flight. I wonder if we have the dependency the wrong way around?
Sorry, might be I am quite a bit tired, but I didn't understand the inversion part.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] andygrove commented on pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
andygrove commented on pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#issuecomment-691512022
@vertexclique I'm closing this for now. Feel free to re-open if this is something that you are still working on.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] vertexclique edited a comment on pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
vertexclique edited a comment on pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#issuecomment-668054877
@andygrove hold on I will open a pr for that too :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] svenwb removed a comment on pull request #7873: ARROW-9609: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
svenwb removed a comment on pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#issuecomment-667080040
+1
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] andygrove closed pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
andygrove closed pull request #7873:
URL: https://github.com/apache/arrow/pull/7873
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] andygrove commented on pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
andygrove commented on pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#issuecomment-691512022
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] vertexclique commented on pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
vertexclique commented on pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#issuecomment-667907467
I really want to merge this PR to get rid of flight and it's dependencies while compiling parquet. My local tests and usage of this version didn't cause any problems and tests were passing. I don't know what is failing at CI at `Dev / Source Release and Merge Script`. Can we merge this one as it is?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] vertexclique commented on pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
vertexclique commented on pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#issuecomment-691518018
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #7873: ARROW-9609: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#issuecomment-667061089
https://issues.apache.org/jira/browse/ARROW-9609
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] vertexclique commented on pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
vertexclique commented on pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#issuecomment-671439482
@andygrove Rebased. Looking out for the CI atm.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] svenwb commented on pull request #7873: ARROW-9609: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
svenwb commented on pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#issuecomment-667080040
+1
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] vertexclique commented on pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
vertexclique commented on pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#issuecomment-668032973
Parquet crate is using arrow's default features which are `default = ["flight", "prettyprint"]` but flight is not needed, nor prettyprint for parquet. these are not used at all.
+ seems like we've added the same dependency as dev dependency. Which is also not needed because it is in the default dependency list.
> ... it seems inverted that arrow depends on flight. I wonder if we have the dependency the wrong way around?
Sorry, might be I am quite a bit tired, but I didn't understand the inversion part.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] vertexclique commented on pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
vertexclique commented on pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#issuecomment-691518018
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] andygrove commented on pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
andygrove commented on pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#issuecomment-668028652
I had a general question ... it seems inverted that arrow depends on flight. I wonder if we have the dependency the wrong way around?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] andygrove commented on pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
andygrove commented on pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#issuecomment-688499002
@vertexclique Just checking in no status. The PR has been idle for a while now. Should we close it?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] vertexclique commented on pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
vertexclique commented on pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#issuecomment-668054877
@andygrove hold on I will open a pr for that too
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] andygrove edited a comment on pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
andygrove edited a comment on pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#issuecomment-688499002
@vertexclique Just checking in on status. The PR has been idle for a while now. Should we close it?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] andygrove commented on pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
andygrove commented on pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#issuecomment-691512022
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] andygrove closed pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
andygrove closed pull request #7873:
URL: https://github.com/apache/arrow/pull/7873
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] andygrove commented on pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
andygrove commented on pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#issuecomment-668066624
@vertexclique I already did, but happy to take yours too.
https://github.com/apache/arrow/pull/7892
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#issuecomment-667261686
https://issues.apache.org/jira/browse/ARROW-9608
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] andygrove commented on pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
andygrove commented on pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#issuecomment-668049019
@vertexclique I will need to look at the code again but I don't see why arrrow would depend on a protocol. I think the protocl should depend on arrow.
The core arrow crate should have as few dependencies as possible, IMO. Same goes for Parquet.
Not everyone will be buiding distributed systems that require the protocol.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] andygrove closed pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
andygrove closed pull request #7873:
URL: https://github.com/apache/arrow/pull/7873
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] vertexclique commented on a change in pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
vertexclique commented on a change in pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#discussion_r463945019
##########
File path: rust/parquet/Cargo.toml
##########
@@ -49,7 +49,6 @@ brotli = "3.3"
flate2 = "1.0"
lz4 = "1.23"
zstd = "0.5"
-arrow = { path = "../arrow", version = "2.0.0-SNAPSHOT" }
Review comment:
yes, because it is already a dependency in `arrow.rs`, using `super::*;` export in test modules will make it use the actual dependency exports of the arrow (which has been done already seems like). Tests had passed locally with no change and it passed here. I see it as good improvement tbh :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] vertexclique commented on pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
vertexclique commented on pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#issuecomment-691518018
@andygrove sorry for the radio silence I was on vacation. Iirc this is not needed anymore. Since we have pruned dependency tree to be leaner. Thanks!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] andygrove commented on pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
andygrove commented on pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#issuecomment-668028237
@vertexclique No, we'll need to fix the regression in the test caused by this change. I can help fix it. I'll look into it later today.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] vertexclique edited a comment on pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
vertexclique edited a comment on pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#issuecomment-668032973
Parquet crate is using arrow's default features which are `default = ["flight", "prettyprint"]` but flight is not needed, nor prettyprint for parquet. these are not used at all. Moreover, since features are additive, we can't override arrow dependencies features in any project that uses parquet if we continue to use default features included.
+ seems like we've added the same dependency as dev dependency. Which is also not needed because it is in the default dependency list.
> ... it seems inverted that arrow depends on flight. I wonder if we have the dependency the wrong way around?
Sorry, might be I am quite a bit tired, but I didn't understand the inversion part.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] vertexclique edited a comment on pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
vertexclique edited a comment on pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#issuecomment-668050755
@andygrove Exactly! This is the problem we are having with the default feature gating scheme that arrow has, which scattered to arrow dependant crates.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] andygrove commented on pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
andygrove commented on pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#issuecomment-668052705
https://issues.apache.org/jira/browse/ARROW-9631
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] sunchao commented on a change in pull request #7873: ARROW-9609: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#discussion_r463729528
##########
File path: rust/parquet/Cargo.toml
##########
@@ -49,7 +49,6 @@ brotli = "3.3"
flate2 = "1.0"
lz4 = "1.23"
zstd = "0.5"
-arrow = { path = "../arrow", version = "2.0.0-SNAPSHOT" }
Review comment:
hmm, does this mean we're removing arrow dependency from tests as well?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] vertexclique commented on pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
vertexclique commented on pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#issuecomment-668050755
@andygrove Exactly! This is definitely the problem we are having with default feature gating scheme that arrow has, which scattered to arrow dependant crates.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] vertexclique commented on pull request #7873: ARROW-9608: [Rust] Leaner feature gating for arrow in parquet
Posted by GitBox <gi...@apache.org>.
vertexclique commented on pull request #7873:
URL: https://github.com/apache/arrow/pull/7873#issuecomment-691518018
@andygrove sorry for the radio silence I was on vacation. Iirc this is not needed anymore. Since we have pruned dependency tree to be leaner. Thanks!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org