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/08/03 15:03:18 UTC

[GitHub] [arrow] vertexclique opened a new pull request #7894: ARROW-9631: [Rust] Make arrow not depend on flight

vertexclique opened a new pull request #7894:
URL: https://github.com/apache/arrow/pull/7894


   reexports flight from arrow if enabled.
   removes unnecessary datafusion deps.
   now by default arrow uses no features. features should be handpicked by the user.


----------------------------------------------------------------
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 #7894: ARROW-9631: [Rust] Make arrow not depend on flight

Posted by GitBox <gi...@apache.org>.
vertexclique commented on pull request #7894:
URL: https://github.com/apache/arrow/pull/7894#issuecomment-670857784


   @andygrove reverted them.


----------------------------------------------------------------
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 #7894: ARROW-9631: [Rust] Make arrow not depend on flight

Posted by GitBox <gi...@apache.org>.
vertexclique edited a comment on pull request #7894:
URL: https://github.com/apache/arrow/pull/7894#issuecomment-671230305


   Hey people, is it possible to merge this? This is currently blocking us together with https://github.com/apache/arrow/pull/7873
   @paddyhoran @nevi-me @andygrove 


----------------------------------------------------------------
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 #7894: ARROW-9631: [Rust] Make arrow not depend on flight

Posted by GitBox <gi...@apache.org>.
andygrove commented on pull request #7894:
URL: https://github.com/apache/arrow/pull/7894#issuecomment-668644692


   We can ignore the mac failure. It's a known issue.


----------------------------------------------------------------
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 #7894: ARROW-9631: [Rust] Make arrow not depend on flight

Posted by GitBox <gi...@apache.org>.
andygrove commented on pull request #7894:
URL: https://github.com/apache/arrow/pull/7894#issuecomment-670575931


   Thanks @vertexclique ... your regex skills are better than mine. 
   
   One last thing ... we probably shouldn't have flight protocol changes as part of this PR. Could you revert those? I'm confused why these are still appearing and this is something I am going to look into next week.


----------------------------------------------------------------
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 #7894: ARROW-9631: [Rust] Make arrow not depend on flight

Posted by GitBox <gi...@apache.org>.
vertexclique commented on pull request #7894:
URL: https://github.com/apache/arrow/pull/7894#issuecomment-668644232


   macos error is odd. since serde_derive is there: https://github.com/apache/arrow/pull/7894/files#diff-c9a039d0f56d7ad3611f9278a44d9056R40


----------------------------------------------------------------
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 #7894: ARROW-9631: [Rust] Make arrow not depend on flight

Posted by GitBox <gi...@apache.org>.
vertexclique edited a comment on pull request #7894:
URL: https://github.com/apache/arrow/pull/7894#issuecomment-671230305


   Hey people, is it possible to merge this? This is currently blocking us together with 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 #7894: ARROW-9631: [Rust] Make arrow not depend on flight

Posted by GitBox <gi...@apache.org>.
andygrove commented on pull request #7894:
URL: https://github.com/apache/arrow/pull/7894#issuecomment-669233005


   @vertexclique I pulled your branch and was able to reproduce the issue by trying to run the datafusion example. The issue is that you changed the datafusion crate to use parquet with no features, so that removed the arrow dependency.
   
   Try this instead in datafusion/Cargo.toml:
   
   ```
   parquet = { path = "../parquet", version = "2.0.0-SNAPSHOT", features = ["arrow"] }
   ```


----------------------------------------------------------------
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 #7894: ARROW-9631: [Rust] Make arrow not depend on flight

Posted by GitBox <gi...@apache.org>.
vertexclique commented on pull request #7894:
URL: https://github.com/apache/arrow/pull/7894#issuecomment-670545087


   @andygrove Seems like done.


----------------------------------------------------------------
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 #7894: ARROW-9631: [Rust] Make arrow not depend on flight

Posted by GitBox <gi...@apache.org>.
vertexclique commented on pull request #7894:
URL: https://github.com/apache/arrow/pull/7894#issuecomment-669776709


   Yes, that's true, enabled it. Resolved.


----------------------------------------------------------------
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 #7894: ARROW-9631: [Rust] Make arrow not depend on flight

Posted by GitBox <gi...@apache.org>.
andygrove commented on pull request #7894:
URL: https://github.com/apache/arrow/pull/7894#issuecomment-668605504


   @vertexclique Please rebase now that the other related PR is merged.


----------------------------------------------------------------
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 #7894: ARROW-9631: [Rust] Make arrow not depend on flight

Posted by GitBox <gi...@apache.org>.
andygrove commented on pull request #7894:
URL: https://github.com/apache/arrow/pull/7894#issuecomment-669969605


   @vertexclique The release scripts now need updating. I spent some time trying to fix them myself this morning but I am out of time for now.
   
   To reproduce locally you can run:
   
   ```
   ./ci/scripts/release_test.sh $(pwd)
   ```
   
   If you look at `00-prepare.sh` you will see it does this:
   
   ```
     sed -i.bak -E \
       -e "s/^version = \".+\"/version = \"${version}\"/g" \
       -e "s/^(arrow = .* version = )\".+\"( .*)/\\1\"${version}\"\\2/g" \
       -e "s/^(arrow-flight = .* version = )\".+\"( .*)/\\1\"${version}\"\\2/g" \
       -e "s/^(parquet = .* version = )\".+\"( .*)/\\1\"${version}\"\\2/g" \
       */Cargo.toml
   ```
   
   Then in `00-prepare-test.rb` it checks for expected results and this is the part that needs updating, to add the feature flags. There are two sections that need updating and they are very similar. Here is one of them:
   
   ```
                      {
                        path: "rust/datafusion/Cargo.toml",
                        hunks: [
                          ["-version = \"#{@snapshot_version}\"",
                           "+version = \"#{@release_version}\""],
                          ["-arrow = { path = \"../arrow\", version = \"#{@snapshot_version}\" }",
                           "-parquet = { path = \"../parquet\", version = \"#{@snapshot_version}\" }",
                           "+arrow = { path = \"../arrow\", version = \"#{@release_version}\" }",
                           "+parquet = { path = \"../parquet\", version = \"#{@release_version}\" }"],
                          ["-arrow-flight = { path = \"../arrow-flight\", version = \"#{@snapshot_version}\" }",
                           "+arrow-flight = { path = \"../arrow-flight\", version = \"#{@release_version}\" }"]
                        ],
                      },
   ```
   
   This is showing the expected diff after changing version numbers, but it no longer matches because of the feature flags.


----------------------------------------------------------------
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 #7894: ARROW-9631: [Rust] Make arrow not depend on flight

Posted by GitBox <gi...@apache.org>.
andygrove closed pull request #7894:
URL: https://github.com/apache/arrow/pull/7894


   


----------------------------------------------------------------
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 #7894: ARROW-9631: [Rust] Make arrow not depend on flight

Posted by GitBox <gi...@apache.org>.
vertexclique commented on pull request #7894:
URL: https://github.com/apache/arrow/pull/7894#issuecomment-669145958


   I don't see these errors when I ran all these tests locally. I am unsure why this is happening. @andygrove 
   ```
   error[E0432]: unresolved import `parquet::arrow`
     --> datafusion\src\execution\physical_plan\parquet.rs:34:14
      |
   34 | use parquet::arrow::{ArrowReader, ParquetFileArrowReader};
      |              ^^^^^ could not find `arrow` in `parquet`
   ```


----------------------------------------------------------------
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 #7894: ARROW-9631: [Rust] Make arrow not depend on flight

Posted by GitBox <gi...@apache.org>.
vertexclique commented on pull request #7894:
URL: https://github.com/apache/arrow/pull/7894#issuecomment-671230305


   Hey people, is it possible to merge this?


----------------------------------------------------------------
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 #7894: ARROW-9631: [Rust] Make arrow not depend on flight

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #7894:
URL: https://github.com/apache/arrow/pull/7894#issuecomment-668073456


   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