You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2025/03/11 12:54:22 UTC
[PR] Better CI feature checks [datafusion]
alamb opened a new pull request, #15156:
URL: https://github.com/apache/datafusion/pull/15156
## Which issue does this PR close?
- Part of https://github.com/apache/datafusion/issues/15155
## Rationale for this change
The coverage for feature flags needs to be improved,
## What changes are included in this PR?
This breaks out the checks for datafusion-substrait and datafusion-proto into their own jobs
I will make a follow on PR for `datafusion-functions` and `datafusion` but I am trying to keep this one reasonably sized
## Are these changes tested?
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code
If tests are not included in your PR, please explain why (for example, are they covered by existing tests)?
-->
## Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!--
If there are any breaking changes to public APIs, please add the `api change` label.
-->
--
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.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Split out `datafusion-substrait` and `datafusion-proto` CI feature checks, increase coverage [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #15156:
URL: https://github.com/apache/datafusion/pull/15156
--
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.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Split out `datafusion-substrait` and `datafusion-proto` CI feature checks [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #15156:
URL: https://github.com/apache/datafusion/pull/15156#discussion_r1989216255
##########
.github/workflows/rust.yml:
##########
@@ -66,9 +66,12 @@ jobs:
# the changes to `Cargo.lock` after building with the updated manifest.
cargo check --profile ci --workspace --all-targets --features integration-tests --locked
- # cargo check common, functions and substrait with no default features
- linux-cargo-check-no-default-features:
- name: cargo check no default features
+ # Check datafusion-common features
+ #
+ # Ensure via `cargo check` that the crate can be built with a
+ # subset of the features packages enabled.
+ linux-datafusion-common-features:
Review Comment:
The diff makes of hard to read but I just moved what jobs each command was run in -- the overall coverage is the same or better
##########
.github/workflows/rust.yml:
##########
@@ -79,28 +82,63 @@ jobs:
uses: ./.github/actions/setup-builder
with:
rust-version: stable
- - name: Check datafusion without default features
- # Some of the test binaries require the parquet feature still
- #run: cargo check --all-targets --no-default-features -p datafusion
- run: cargo check --profile ci --no-default-features -p datafusion
-
- - name: Check datafusion-common without default features
+ - name: Check datafusion-common (no-default-features)
run: cargo check --profile ci --all-targets --no-default-features -p datafusion-common
+ # Note: don't check other feature flags as datafusion-common is not typically used standalone
- - name: Check datafusion-functions without default features
- run: cargo check --profile ci --all-targets --no-default-features -p datafusion-functions
-
- - name: Check datafusion-substrait without default features
+ # Check datafusion-substrait features
+ #
+ # Ensure via `cargo check` that the crate can be built with a
+ # subset of the features packages enabled.
+ linux-datafusion-substrait-features:
+ name: cargo check datafusion-substrait features
+ needs: linux-build-lib
+ runs-on: ubuntu-latest
+ container:
+ image: amd64/rust
+ steps:
+ - uses: actions/checkout@v4
+ - name: Setup Rust toolchain
+ uses: ./.github/actions/setup-builder
+ with:
+ rust-version: stable
+ - name: Check datafusion-substrait (no-default-features)
run: cargo check --profile ci --all-targets --no-default-features -p datafusion-substrait
+ - name: Check datafusion-substrait (physical)
Review Comment:
this adds coverage of compiling datafusion-substrait with the available feature flags
--
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.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Split out `datafusion-substrait` and `datafusion-proto` CI feature checks [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #15156:
URL: https://github.com/apache/datafusion/pull/15156#discussion_r1989360930
##########
.github/workflows/rust.yml:
##########
@@ -79,28 +82,68 @@ jobs:
uses: ./.github/actions/setup-builder
with:
rust-version: stable
- - name: Check datafusion without default features
- # Some of the test binaries require the parquet feature still
- #run: cargo check --all-targets --no-default-features -p datafusion
- run: cargo check --profile ci --no-default-features -p datafusion
-
- - name: Check datafusion-common without default features
+ - name: Check datafusion-common (no-default-features)
run: cargo check --profile ci --all-targets --no-default-features -p datafusion-common
+ # Note: don't check other feature flags as datafusion-common is not typically used standalone
- - name: Check datafusion-functions without default features
- run: cargo check --profile ci --all-targets --no-default-features -p datafusion-functions
-
- - name: Check datafusion-substrait without default features
+ # Check datafusion-substrait features
+ #
+ # Ensure via `cargo check` that the crate can be built with a
+ # subset of the features packages enabled.
+ linux-datafusion-substrait-features:
+ name: cargo check datafusion-substrait features
+ needs: linux-build-lib
+ runs-on: ubuntu-latest
+ container:
+ image: amd64/rust
+ steps:
+ - uses: actions/checkout@v4
+ - name: Setup Rust toolchain
+ uses: ./.github/actions/setup-builder
+ with:
+ rust-version: stable
+ - name: Check datafusion-substrait (no-default-features)
run: cargo check --profile ci --all-targets --no-default-features -p datafusion-substrait
+ - name: Check datafusion-substrait (physical)
+ run: cargo check --profile ci --all-targets --no-default-features -p datafusion-substrait --features=physical
+ - name: Install Protobuf Compiler
+ run: |
+ apt-get update
+ apt-get install -y protobuf-compiler
+ - name: Check datafusion-substrait (protoc)
+ run: cargo check --profile ci --all-targets --no-default-features -p datafusion-substrait --features=protoc
- - name: Check workspace in debug mode
- run: cargo check --profile ci --all-targets --workspace
-
- - name: Check workspace with additional features
- run: cargo check --profile ci --workspace --benches --features avro,json,integration-tests
-
- # cargo check datafusion to ensure that the datafusion crate can be built with only a
- # subset of the function packages enabled.
+ # Check datafusion-proto features
+ #
+ # Ensure via `cargo check` that the crate can be built with a
+ # subset of the features packages enabled.
+ linux-datafusion-proto-features:
+ name: cargo check datafusion-proto features
+ needs: linux-build-lib
+ runs-on: ubuntu-latest
+ container:
+ image: amd64/rust
+ steps:
+ - uses: actions/checkout@v4
+ - name: Setup Rust toolchain
+ uses: ./.github/actions/setup-builder
+ with:
+ rust-version: stable
+ - name: Check datafusion-proto (no-default-features)
+ run: cargo check --profile ci --all-targets --no-default-features -p datafusion-proto
+ # fails due to https://github.com/apache/datafusion/issues/15157
Review Comment:
This adds coverage for the datafusion-proto crate, and in fact found a bug:
- https://github.com/apache/datafusion/issues/15157
--
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.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Split out `datafusion-substrait` and `datafusion-proto` CI feature checks, increase coverage [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #15156:
URL: https://github.com/apache/datafusion/pull/15156#issuecomment-2714973347
Thank you for the review @timsaucer !
--
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.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org