-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[CI] Refactor CI builders #8826
Conversation
@Jefffrey please review once you have time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍
# Set debuginfo=line-tables-only as debuginfo=0 causes immensely slow build | ||
# See for more details: https://github.com/rust-lang/rust/issues/119560 | ||
# | ||
# set RUST_MIN_STACK to avoid rust stack overflows on tpc-ds tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe put this as a TODO item, since ideally we'd want the tests to pass without needing this setting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still need it as TPC-DS tests spontaneously fail on stack overflow. But we prob need to check why is it happening. That might be a bigger problem with piling up the stackframes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree it would be great to file a ticket to track the stack overflow
https://github.com/apache/arrow-datafusion/issues?q=is%3Aissue+is%3Aopen+stack+overflow doesn't show any obvious candidate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filed #8837
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reviewed https://github.com/apache/arrow-datafusion/actions/runs/7491755676/job/20393701106?pr=8826 and all the tests look good to me
Thank you @comphead
# Set debuginfo=line-tables-only as debuginfo=0 causes immensely slow build | ||
# See for more details: https://github.com/rust-lang/rust/issues/119560 | ||
# | ||
# set RUST_MIN_STACK to avoid rust stack overflows on tpc-ds tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree it would be great to file a ticket to track the stack overflow
https://github.com/apache/arrow-datafusion/issues?q=is%3Aissue+is%3Aopen+stack+overflow doesn't show any obvious candidate
Co-authored-by: Andrew Lamb <[email protected]>
Co-authored-by: Andrew Lamb <[email protected]>
Which issue does this PR close?
Closes #.
Rationale for this change
rust.yml
contains a lot of duplicates and a little bit overcomplicated, it would be nice to simplify the main CI fileWhat changes are included in this PR?
PR idea is to reduce complexity for
rust.yml
and factor out common properties like Rust into a single builder. Separately introduced other builders for each os.Are these changes tested?
Are there any user-facing changes?