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

Fix compilation for ballista in stand-alone mode #1008

Merged
merged 1 commit into from
Sep 18, 2021

Conversation

Igosuki
Copy link
Contributor

@Igosuki Igosuki commented Sep 16, 2021

Which issue does this PR close?

Closes #1000

Rationale for this change

API changes broke compilation and execution of the standalone ballista client.

What changes are included in this PR?

Compilation fixes, and another fix is required for tests to pass

Are there any user-facing changes?

None

@Igosuki
Copy link
Contributor Author

Igosuki commented Sep 16, 2021

Problem here https://github.com/apache/arrow-datafusion/blob/master/ballista/rust/executor/src/standalone.rs#L55
By the time df collects test results the tonic server is unreachable, I tried to make it run ad-vitam but then the tests never end.

@alamb alamb changed the title Fix compilation Fix compilation for ballista in stand-alone mode Sep 17, 2021
@alamb
Copy link
Contributor

alamb commented Sep 17, 2021

I verified that the cargo check --features=standalone compiles on this branch, but does not on master

When we fix the ballista test, I recommend we start running the standalone client tests as part of CI so they aren't broken again

@alamb
Copy link
Contributor

alamb commented Sep 18, 2021

This seems like an improvement to me, so merging it in. We can always improve things more in the future

@alamb
Copy link
Contributor

alamb commented Sep 18, 2021

Thanks again @Igosuki

@alamb alamb merged commit a640390 into apache:master Sep 18, 2021
@Igosuki
Copy link
Contributor Author

Igosuki commented Sep 18, 2021

So how do we fix the ballista test though ? I don't understand why the tonic server is unreachable since even if the thread gets detached because the tokio JoinHandle gets dropped it should still be running right ?

@alamb
Copy link
Contributor

alamb commented Sep 19, 2021

I wonder which part of the not is not parsing.

Sorry @Igosuki I am not super familiar with the ballista codebase yet. What test are you referring to (aka what command are you running locally that is failing)?

With that information I can file a ticket for the problem and probably find someone in the community who can help fix it

@Igosuki
Copy link
Contributor Author

Igosuki commented Sep 19, 2021

@alamb This one https://github.com/apache/arrow-datafusion/blob/master/ballista/rust/client/src/context.rs#L300

@alamb
Copy link
Contributor

alamb commented Sep 19, 2021

Filed #1020 to track test failing

@houqp houqp added the bug Something isn't working label Nov 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ballista client fails to build with --features=standalone
3 participants