Skip to content

Use latest DataFusion - #86

Merged
andygrove merged 5 commits into
apache:masterfrom
yahoNanJing:issue-84
Jul 13, 2022
Merged

andygrove merged 5 commits into
apache:masterfrom
yahoNanJing:issue-84

Conversation

@yahoNanJing

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Closes #84.

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

@yahoNanJing

Copy link
Copy Markdown
Contributor Author

Hi @alamb and @andygrove, could you help review this PR?

@andygrove

Copy link
Copy Markdown
Member

I tried running the integration tests from this branch, by running ./dev/integration-tests.sh, and they failed with:

error: failed to select a version for the requirement `parquet = "^18.0.0"`
candidate versions found which didn't match: 15.0.0, 14.0.0, 13.0.0, ...

@yahoNanJing

Copy link
Copy Markdown
Contributor Author

Thanks @andygrove. I didn't run the test. I'll fix it.

@andygrove andygrove left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I had to update the ballista-base dockerfile to use a newer Rust version and then the integration tests ran:

FROM rust:1.62.0-buster AS builder

I don't really understand why this helped though.

@yahoNanJing

Copy link
Copy Markdown
Contributor Author

The rust version on my Mac is also 1.62.0. Maybe we need to update the rust version in the toml as well later. By the way, really appreciate the docker-based integration tests.

@andygrove

Copy link
Copy Markdown
Member

I will merge in the next couple days if there is no more feedback.

@thinkharderdev @Ted-Jiang - do you want to review?

@thinkharderdev thinkharderdev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@andygrove
andygrove merged commit 56ec4df into apache:master Jul 13, 2022

@alamb alamb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

epic work @yahoNanJing ❤️ It is cool to see the net deletion of code

@Ted-Jiang Ted-Jiang left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍👍👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Upgrade dependency of arrow-datafusion to commit d0d5564b8f689a01e542b8c1df829d74d0fab2b0

5 participants