Skip to content

Split out datafusion-substrait and datafusion-proto CI feature checks, increase coverage #15156

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Mar 12, 2025
Merged

Split out datafusion-substrait and datafusion-proto CI feature checks, increase coverage #15156

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
631e0bf
Better CI feature checks
alamb Mar 11, 2025
5088785
adjust feature
alamb Mar 11, 2025
cafd428
Install protobuf compiler for checking substrait/protoc
alamb Mar 11, 2025
1c7ffc0
comment out failing test
alamb Mar 11, 2025
12c1236
fix
alamb Mar 11, 2025
90839fd
fix protoc
alamb Mar 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 76 additions & 23 deletions .github/workflows/rust.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

name: cargo check datafusion-common features
needs: linux-build-lib
runs-on: ubuntu-latest
container:
Expand All @@ -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)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this adds coverage of compiling datafusion-substrait with the available feature flags

run: cargo check --profile ci --all-targets --no-default-features -p datafusion-substrait --features=physical
- name: Install cmake
run: |
# note the builder setup runs apt-get update / installs protobuf compiler
apt-get install -y cmake
- 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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This adds coverage for the datafusion-proto crate, and in fact found a bug:

#- name: Check datafusion-proto (json)
# run: cargo check --profile ci --all-targets --no-default-features -p datafusion-proto --features=json
- name: Check datafusion-proto (parquet)
run: cargo check --profile ci --all-targets --no-default-features -p datafusion-proto --features=parquet
- name: Check datafusion-proto (avro)
run: cargo check --profile ci --all-targets --no-default-features -p datafusion-proto --features=avro


# Check datafusion crate features
#
# Ensure via `cargo check` that the crate can be built with a
# subset of the features packages enabled.
linux-cargo-check-datafusion:
name: cargo check datafusion
needs: linux-build-lib
Expand All @@ -113,6 +156,11 @@ jobs:
uses: ./.github/actions/setup-builder
with:
rust-version: stable
- name: Check datafusion (no-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 (nested_expressions)
run: cargo check --profile ci --no-default-features --features=nested_expressions -p datafusion

Expand All @@ -134,8 +182,10 @@ jobs:
- name: Check datafusion (string_expressions)
run: cargo check --profile ci --no-default-features --features=string_expressions -p datafusion

# cargo check datafusion-functions to ensure that the datafusion-functions crate can be built with
# only a subset of the function packages enabled.
# Check datafusion-functions crate features
#
# Ensure via `cargo check` that the crate can be built with a
# subset of the features packages enabled.
linux-cargo-check-datafusion-functions:
name: cargo check functions
needs: linux-build-lib
Expand All @@ -148,6 +198,9 @@ jobs:
uses: ./.github/actions/setup-builder
with:
rust-version: stable
- name: Check datafusion-functions (no-default-features)
run: cargo check --profile ci --all-targets --no-default-features -p datafusion-functions

- name: Check datafusion-functions (crypto)
run: cargo check --profile ci --all-targets --no-default-features --features=crypto_expressions -p datafusion-functions

Expand Down
Loading