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 2021/05/01 11:13:18 UTC

[GitHub] [arrow-datafusion] Dandandan opened a new pull request #231: Move datafusion cli

Dandandan opened a new pull request #231:
URL: https://github.com/apache/arrow-datafusion/pull/231


   # Which issue does this PR close?
   
   Closes #218
   
    # Rationale for this change
   * Building DataFusion fetches extra dependencies (`clap` `rustyline` and transitive dependencies) unneeded for the rest of DataFusion, slowing the development cycle down a bit (mainly for clean builds).
   * Installing the binary using cargo is a bit more cumbersome, `cargo install datafusion --bin datafusion-cli --release`
   * dependency constraints on the cli dependencies potentially can cause conflicts for DataFusion consumers
   
   # What changes are included in this PR?
   
   * Moves the source code & dependencies to datafusion-cli
   * Drive-by change to make the default batch size a more reasonable value (8192 instead of >1M)
   
   # Are there any user-facing changes?
   
   Users should be able to use datafusion-cli now instead of using the `bin` option inside of datafusion.


-- 
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-datafusion] andygrove commented on a change in pull request #231: Move datafusion-cli to new crate

Posted by GitBox <gi...@apache.org>.
andygrove commented on a change in pull request #231:
URL: https://github.com/apache/arrow-datafusion/pull/231#discussion_r624509799



##########
File path: datafusion/docs/cli.md
##########
@@ -26,19 +26,19 @@ The DataFusion CLI is a command-line interactive SQL utility that allows queries
 Use the following commands to clone this repository and run the CLI. This will require the Rust toolchain to be installed. Rust can be installed from [https://rustup.rs/](https://rustup.rs/).
 
 ```sh
-git clone https://github.com/apache/arrow
-cd arrow/rust/datafusion
-cargo run --bin datafusion-cli --release
+git clone https://github.com/apache/arrow-datafusion
+cd arrow/datafusion-cli

Review comment:
       ```suggestion
   cd arrow-datafusion/datafusion-cli
   ```




-- 
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-datafusion] Dandandan commented on pull request #231: Move datafusion-cli to new crate

Posted by GitBox <gi...@apache.org>.
Dandandan commented on pull request #231:
URL: https://github.com/apache/arrow-datafusion/pull/231#issuecomment-830641261


   > The Dockerfile will also need to be updated (it looks like it has incorrect paths). The Dockerfile also probably needs moving to the new datafusion-cli folder too.
   
   Thanks, I updated the Dockerfile


-- 
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-datafusion] andygrove commented on pull request #231: Move datafusion-cli to new crate

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


   LGTM. I can't approve this PR for some reason. Maybe a GitHub glitch .. I will check again later.


-- 
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-datafusion] codecov-commenter edited a comment on pull request #231: Move datafusion-cli to new crate

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #231:
URL: https://github.com/apache/arrow-datafusion/pull/231#issuecomment-830620117


   # [Codecov](https://codecov.io/gh/apache/arrow-datafusion/pull/231?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#231](https://codecov.io/gh/apache/arrow-datafusion/pull/231?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (220fcf0) into [master](https://codecov.io/gh/apache/arrow-datafusion/commit/23d02bb3c642ed69e7b963ed74df9687b91af970?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (23d02bb) will **increase** coverage by `0.00%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/arrow-datafusion/pull/231/graphs/tree.svg?width=650&height=150&src=pr&token=JXwWBKD3D9&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/arrow-datafusion/pull/231?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master     #231   +/-   ##
   =======================================
     Coverage   76.46%   76.46%           
   =======================================
     Files         135      134    -1     
     Lines       23250    23249    -1     
   =======================================
     Hits        17777    17777           
   + Misses       5473     5472    -1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/arrow-datafusion/pull/231?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [datafusion-cli/src/main.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/231/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi1jbGkvc3JjL21haW4ucnM=) | `0.00% <0.00%> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-datafusion/pull/231?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/arrow-datafusion/pull/231?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [23d02bb...220fcf0](https://codecov.io/gh/apache/arrow-datafusion/pull/231?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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-datafusion] andygrove commented on a change in pull request #231: Move datafusion-cli to new crate

Posted by GitBox <gi...@apache.org>.
andygrove commented on a change in pull request #231:
URL: https://github.com/apache/arrow-datafusion/pull/231#discussion_r624509746



##########
File path: datafusion-cli/src/main.rs
##########
@@ -44,7 +44,7 @@ pub async fn main() {
         )
         .arg(
             Arg::with_name("batch-size")
-                .help("The batch size of each query, default value is 1048576")

Review comment:
       We could get this value programmatically by creating a default config and getting this value.




-- 
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-datafusion] Dandandan commented on a change in pull request #231: Move datafusion-cli to new crate

Posted by GitBox <gi...@apache.org>.
Dandandan commented on a change in pull request #231:
URL: https://github.com/apache/arrow-datafusion/pull/231#discussion_r624509795



##########
File path: datafusion-cli/src/main.rs
##########
@@ -44,7 +44,7 @@ pub async fn main() {
         )
         .arg(
             Arg::with_name("batch-size")
-                .help("The batch size of each query, default value is 1048576")

Review comment:
       :+1: agreed




-- 
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-datafusion] codecov-commenter edited a comment on pull request #231: Move datafusion-cli to new crate

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #231:
URL: https://github.com/apache/arrow-datafusion/pull/231#issuecomment-830620117


   # [Codecov](https://codecov.io/gh/apache/arrow-datafusion/pull/231?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#231](https://codecov.io/gh/apache/arrow-datafusion/pull/231?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (27419be) into [master](https://codecov.io/gh/apache/arrow-datafusion/commit/23d02bb3c642ed69e7b963ed74df9687b91af970?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (23d02bb) will **increase** coverage by `0.00%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/arrow-datafusion/pull/231/graphs/tree.svg?width=650&height=150&src=pr&token=JXwWBKD3D9&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/arrow-datafusion/pull/231?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master     #231   +/-   ##
   =======================================
     Coverage   76.46%   76.46%           
   =======================================
     Files         135      134    -1     
     Lines       23250    23249    -1     
   =======================================
     Hits        17777    17777           
   + Misses       5473     5472    -1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/arrow-datafusion/pull/231?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [datafusion-cli/src/main.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/231/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi1jbGkvc3JjL21haW4ucnM=) | `0.00% <0.00%> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-datafusion/pull/231?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/arrow-datafusion/pull/231?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [23d02bb...27419be](https://codecov.io/gh/apache/arrow-datafusion/pull/231?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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-datafusion] andygrove commented on pull request #231: Move datafusion-cli to new crate

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


   The Dockerfile will also need to be updated (it looks like it has incorrect paths). The Dockerfile also probably needs moving to the new datafusion-cli folder 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-datafusion] andygrove commented on a change in pull request #231: Move datafusion-cli to new crate

Posted by GitBox <gi...@apache.org>.
andygrove commented on a change in pull request #231:
URL: https://github.com/apache/arrow-datafusion/pull/231#discussion_r624509824



##########
File path: datafusion/docs/cli.md
##########
@@ -26,19 +26,19 @@ The DataFusion CLI is a command-line interactive SQL utility that allows queries
 Use the following commands to clone this repository and run the CLI. This will require the Rust toolchain to be installed. Rust can be installed from [https://rustup.rs/](https://rustup.rs/).
 
 ```sh
-git clone https://github.com/apache/arrow
-cd arrow/rust/datafusion
-cargo run --bin datafusion-cli --release
+git clone https://github.com/apache/arrow-datafusion
+cd arrow/datafusion-cli
+cargo run --release
 ```
 
 ## Run using Docker
 
 Use the following commands to clone this repository and build a Docker image containing the CLI tool. Note that there is `.dockerignore` file in the root of the repository that may need to be deleted in order for this to work.
 
 ```sh
-git clone https://github.com/apache/arrow
+git clone https://github.com/apache/arrow-datafusion
 cd arrow

Review comment:
       ```suggestion
   cd arrow-datafusion
   ```




-- 
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-datafusion] Dandandan commented on a change in pull request #231: Move datafusion-cli to new crate

Posted by GitBox <gi...@apache.org>.
Dandandan commented on a change in pull request #231:
URL: https://github.com/apache/arrow-datafusion/pull/231#discussion_r624511744



##########
File path: datafusion-cli/src/main.rs
##########
@@ -44,7 +44,7 @@ pub async fn main() {
         )
         .arg(
             Arg::with_name("batch-size")
-                .help("The batch size of each query, default value is 1048576")

Review comment:
       I made it inherit the DataFusion default now.




-- 
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-datafusion] codecov-commenter commented on pull request #231: Move datafusion cli to new crate

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #231:
URL: https://github.com/apache/arrow-datafusion/pull/231#issuecomment-830620117


   # [Codecov](https://codecov.io/gh/apache/arrow-datafusion/pull/231?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#231](https://codecov.io/gh/apache/arrow-datafusion/pull/231?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (280d0a4) into [master](https://codecov.io/gh/apache/arrow-datafusion/commit/23d02bb3c642ed69e7b963ed74df9687b91af970?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (23d02bb) will **increase** coverage by `0.00%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/arrow-datafusion/pull/231/graphs/tree.svg?width=650&height=150&src=pr&token=JXwWBKD3D9&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/arrow-datafusion/pull/231?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master     #231   +/-   ##
   =======================================
     Coverage   76.46%   76.46%           
   =======================================
     Files         135      134    -1     
     Lines       23250    23248    -2     
   =======================================
     Hits        17777    17777           
   + Misses       5473     5471    -2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/arrow-datafusion/pull/231?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [datafusion-cli/src/main.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/231/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi1jbGkvc3JjL21haW4ucnM=) | `0.00% <ø> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-datafusion/pull/231?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/arrow-datafusion/pull/231?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [23d02bb...280d0a4](https://codecov.io/gh/apache/arrow-datafusion/pull/231?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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