Skip to content
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

Move datafusion-cli to new crate #231

Merged
merged 12 commits into from
May 3, 2021
Merged

Conversation

Dandandan
Copy link
Contributor

@Dandandan Dandandan commented May 1, 2021

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 currently a bit more cumbersome, cargo install datafusion --bin datafusion-cli --release
  • dependency constraints on the cli dependencies potentially can cause conflicts for DataFusion consumers
Version Nr dependencies
Master 199
PR 186

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)
  • Documentation updates
  • Update rustyline version

Are there any user-facing changes?

Users should be able to use datafusion-cli now instead of using the bin option inside of datafusion.

@Dandandan Dandandan changed the title Move datafusion cli Move datafusion cli to new crate May 1, 2021
@codecov-commenter
Copy link

codecov-commenter commented May 1, 2021

Codecov Report

Merging #231 (220fcf0) into master (23d02bb) will increase coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@           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 Coverage Δ
datafusion-cli/src/main.rs 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 23d02bb...220fcf0. Read the comment docs.

@Dandandan Dandandan changed the title Move datafusion cli to new crate Move datafusion-cli to new crate May 1, 2021
datafusion/docs/cli.md Outdated Show resolved Hide resolved
datafusion/docs/cli.md Outdated Show resolved Hide resolved
@andygrove
Copy link
Member

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.

@Dandandan
Copy link
Contributor Author

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

@andygrove
Copy link
Member

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

@alamb
Copy link
Contributor

alamb commented May 3, 2021

LGTM too. Thanks @Dandandan

@alamb alamb merged commit 47bd3fa into apache:master May 3, 2021
@alamb alamb added the datafusion Changes in the datafusion crate label May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
datafusion Changes in the datafusion crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move DataFusion cli/repl to own crate
4 participants