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