You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "Huy1Ng (via GitHub)" <gi...@apache.org> on 2025/06/29 00:07:59 UTC
[PR] improve rust workflows without cache [datafusion-ballista]
Huy1Ng opened a new pull request, #1275:
URL: https://github.com/apache/datafusion-ballista/pull/1275
# Which issue does this PR close?
Part of #1128.
# Rationale for this change
In the issue
# What changes are included in this PR?
Improve the rust workflows by following the practices in parent datafusion repo (with couple inspiration from popular rust project like uv or polars). Changes are:
- Parallelize all jobs. I don't see any reason to wait for linux-build-lib to complete first before running other jobs
- Move prep steps out into actions
- Use cargo check instead of cargo build
- Remove caches altogether. While caches help a lot for a particular job, the compiler couldn't reuse other jobs' artifacts, so dependent jobs will always re-download and recompile their own => take up lots of cache storage space. Also, cache is not commonly used in other projects that I researched.
# Are there any user-facing changes?
No
--
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] improve rust workflows without cache [datafusion-ballista]
Posted by "Huy1Ng (via GitHub)" <gi...@apache.org>.
Huy1Ng commented on code in PR #1275:
URL: https://github.com/apache/datafusion-ballista/pull/1275#discussion_r2217684707
##########
.github/actions/setup-builder/action.yaml:
##########
@@ -37,3 +37,5 @@ runs:
rustup toolchain install ${{ inputs.rust-version }}
Review Comment:
I tried but the current repo settings doesn't allow `dtolnay/rust-toolchain@stable`. `datafusion` repo seems to have the same limitations: https://github.com/apache/datafusion/pull/15315.
--
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] improve rust workflows without cache [datafusion-ballista]
Posted by "Huy1Ng (via GitHub)" <gi...@apache.org>.
Huy1Ng commented on PR #1275:
URL: https://github.com/apache/datafusion-ballista/pull/1275#issuecomment-3083153716
- I reused the workflows from datafusion repo. `cancel` was removed in this commit: https://github.com/apache/datafusion/commit/0820eb987ff9555b48a0e1704f5a8644ea4ab087#diff-2d26dd5833955d0dacfcaff96648d690f830e454548cf9832ae5fa68404012f2 . It's now replaced with concurrency groups.
- I will put it back in
--
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] improve rust workflows without cache [datafusion-ballista]
Posted by "milenkovicm (via GitHub)" <gi...@apache.org>.
milenkovicm commented on PR #1275:
URL: https://github.com/apache/datafusion-ballista/pull/1275#issuecomment-3123962588
thanks againn @Huy1Ng
--
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] improve rust workflows without cache [datafusion-ballista]
Posted by "Huy1Ng (via GitHub)" <gi...@apache.org>.
Huy1Ng commented on code in PR #1275:
URL: https://github.com/apache/datafusion-ballista/pull/1275#discussion_r2217686513
##########
.github/workflows/rust.yml:
##########
@@ -17,275 +17,151 @@
name: Rust
+concurrency:
+ group: ${{ github.repository }}-${{ github.workflow }}
+ cancel-in-progress: true
+
on:
# always trigger
push:
pull_request:
jobs:
- # build the library, a compilation step used by multiple steps below
+ # Check crate compiles and base cargo check passes
linux-build-lib:
- name: Build Libraries on AMD64 Rust ${{ matrix.rust }}
+ name: linux build test
runs-on: ubuntu-latest
- strategy:
- matrix:
- arch: [amd64]
- rust: [stable]
container:
- image: ${{ matrix.arch }}/rust
- env:
- # Disable full debug symbol generation to speed up CI build and keep memory down
- # "1" means line tables only, which is useful for panic tracebacks.
- RUSTFLAGS: "-C debuginfo=1"
+ image: amd64/rust
steps:
- uses: actions/checkout@v4
- - name: Cache Cargo
- uses: actions/cache@v4
- with:
- # these represent dependencies downloaded by cargo
- # and thus do not depend on the OS, arch nor rust version.
- path: /github/home/.cargo
- key: cargo-cache-
- - name: Cache Rust dependencies
- uses: actions/cache@v4
with:
- # these represent compiled steps of both dependencies and arrow
- # and thus are specific for a particular OS, arch and rust version.
- path: /github/home/target
- key: ${{ runner.os }}-${{ matrix.arch }}-target-cache-${{ matrix.rust }}-
+ submodules: true
- name: Setup Rust toolchain
uses: ./.github/actions/setup-builder
- with:
- rust-version: ${{ matrix.rust }}
- - name: Build workspace in release mode
+ - name: Prepare cargo build
run: |
- cargo build --release
- ls -l /github/home/target/release
- env:
- CARGO_HOME: "/github/home/.cargo"
- CARGO_TARGET_DIR: "/github/home/target"
- - name: Save artifacts
- uses: actions/upload-artifact@v4
- with:
- name: rust-artifacts
- path: |
- /github/home/target/release/ballista-scheduler
- /github/home/target/release/ballista-executor
+ # Adding `--locked` here to assert that the `Cargo.lock` file is up to
+ # date with the manifest. When this fails, please make sure to commit
+ # the changes to `Cargo.lock` after building with the updated manifest.
+ cargo check --profile ci --workspace --all-targets --locked
- # test the crate
linux-test:
- name: Test Workspace on AMD64 Rust ${{ matrix.rust }}
- needs: [linux-build-lib]
+ name: test linux crates
runs-on: ubuntu-latest
- strategy:
- matrix:
- arch: [amd64]
- rust: [stable]
container:
- image: ${{ matrix.arch }}/rust
- env:
- # Disable full debug symbol generation to speed up CI build and keep memory down
- # "1" means line tables only, which is useful for panic tracebacks.
- RUSTFLAGS: "-C debuginfo=1"
+ image: amd64/rust
steps:
- uses: actions/checkout@v4
with:
submodules: true
- - name: Install protobuf compiler
- shell: bash
- run: |
- apt-get -qq update && apt-get -y -qq install protobuf-compiler
- protoc --version
- - name: Cache Cargo
- uses: actions/cache@v4
- with:
- path: /github/home/.cargo
- # this key equals the ones on `linux-build-lib` for re-use
- key: cargo-cache-
- - name: Cache Rust dependencies
- uses: actions/cache@v4
- with:
- path: /github/home/target
- # this key equals the ones on `linux-build-lib` for re-use
- key: ${{ runner.os }}-${{ matrix.arch }}-target-cache-${{ matrix.rust }}
- name: Setup Rust toolchain
uses: ./.github/actions/setup-builder
- with:
- rust-version: ${{ matrix.rust }}
- name: Run tests
run: |
export PATH=$PATH:$HOME/d/protoc/bin
export ARROW_TEST_DATA=$(pwd)/testing/data
export PARQUET_TEST_DATA=$(pwd)/parquet-testing/data
- cargo test --features=testcontainers
+ cargo test --profile ci --features=testcontainers
env:
CARGO_HOME: "/github/home/.cargo"
CARGO_TARGET_DIR: "/github/home/target"
+
+ linux-build-workspace:
+ name: check linux workspace
+ runs-on: ubuntu-latest
+ container:
+ image: amd64/rust
+ steps:
+ - uses: actions/checkout@v4
+ with:
+ submodules: true
+ - name: Setup Rust toolchain
+ uses: ./.github/actions/setup-builder
- name: Try to compile when `--no-default-features` is selected
run: |
export PATH=$PATH:$HOME/d/protoc/bin
export ARROW_TEST_DATA=$(pwd)/testing/data
export PARQUET_TEST_DATA=$(pwd)/parquet-testing/data
- cargo build -p ballista-scheduler -p ballista-executor -p ballista-core -p ballista --no-default-features
+ cargo check --profile ci -p ballista-scheduler -p ballista-executor -p ballista-core -p ballista --no-default-features --locked
env:
CARGO_HOME: "/github/home/.cargo"
CARGO_TARGET_DIR: "/github/home/target"
- # run ballista tests
ballista-test:
- name: Test Ballista on AMD64 Rust ${{ matrix.rust }}
- needs: [linux-build-lib]
+ name: test linux balista
runs-on: ubuntu-latest
- strategy:
- matrix:
- arch: [amd64]
- rust: [stable]
container:
- image: ${{ matrix.arch }}/rust
- env:
- # Disable full debug symbol generation to speed up CI build and keep memory down
- # "1" means line tables only, which is useful for panic tracebacks.
- RUSTFLAGS: "-C debuginfo=1"
+ image: amd64/rust
steps:
- uses: actions/checkout@v4
with:
submodules: true
- - name: Install protobuf compiler
- shell: bash
- run: |
- mkdir -p $HOME/d/protoc
- cd $HOME/d/protoc
- export PROTO_ZIP="protoc-21.4-linux-x86_64.zip"
- curl -LO https://github.com/protocolbuffers/protobuf/releases/download/v21.4/$PROTO_ZIP
- unzip $PROTO_ZIP
- export PATH=$PATH:$HOME/d/protoc/bin
- protoc --version
- - name: Cache Cargo
- uses: actions/cache@v4
- with:
- path: /github/home/.cargo
- # this key equals the ones on `linux-build-lib` for re-use
- key: cargo-cache-
- - name: Cache Rust dependencies
- uses: actions/cache@v4
- with:
- path: /github/home/target
- # this key equals the ones on `linux-build-lib` for re-use
- key: ${{ runner.os }}-${{ matrix.arch }}-target-cache-${{ matrix.rust }}
- name: Setup Rust toolchain
uses: ./.github/actions/setup-builder
- with:
- rust-version: ${{ matrix.rust }}
- # Ballista is currently not part of the main workspace so requires a separate test step
- name: Run Ballista tests
run: |
export PATH=$PATH:$HOME/d/protoc/bin
export ARROW_TEST_DATA=$(pwd)/testing/data
export PARQUET_TEST_DATA=$(pwd)/parquet-testing/data
cd ballista
# Ensure also compiles in standalone mode
- cargo test --no-default-features --features standalone
+ cargo test --profile ci --no-default-features --features standalone --locked
env:
CARGO_HOME: "/github/home/.cargo"
CARGO_TARGET_DIR: "/github/home/target"
- windows-and-macos:
- name: Test on ${{ matrix.os }} Rust ${{ matrix.rust }}
- runs-on: ${{ matrix.os }}
- strategy:
- matrix:
- os: [windows-latest, macos-latest]
- rust: [stable]
+ windows-test:
+ name: windows test
+ runs-on: windows-latest
steps:
- uses: actions/checkout@v4
with:
submodules: true
- - name: Install protobuf macos compiler
- shell: bash
- run: |
- mkdir -p $HOME/d/protoc
- cd $HOME/d/protoc
- export PROTO_ZIP="protoc-21.4-osx-x86_64.zip"
- curl -LO https://github.com/protocolbuffers/protobuf/releases/download/v21.4/$PROTO_ZIP
- unzip $PROTO_ZIP
- echo "$HOME/d/protoc/bin" >> $GITHUB_PATH
- export PATH=$PATH:$HOME/d/protoc/bin
- protoc --version
- if: ${{matrix.os == 'macos-latest'}}
- - name: Install protobuf windows compiler
+ - name: Setup Rust toolchain
+ uses: ./.github/actions/setup-windows-builder
+ - name: Run tests
shell: bash
run: |
- mkdir -p $HOME/d/protoc
- cd $HOME/d/protoc
- export PROTO_ZIP="protoc-21.4-win64.zip"
- curl -LO https://github.com/protocolbuffers/protobuf/releases/download/v21.4/$PROTO_ZIP
- unzip $PROTO_ZIP
export PATH=$PATH:$HOME/d/protoc/bin
- protoc.exe --version
- if: ${{matrix.os == 'windows-latest'}}
- # TODO: this won't cache anything, which is expensive. Setup this action
- # with a OS-dependent path.
+ export ARROW_TEST_DATA=$(pwd)/testing/data
+ export PARQUET_TEST_DATA=$(pwd)/parquet-testing/data
+ export TMP=$(pwd)/.temp
+ mkdir -p "$TMP"
+ cargo test --profile ci --locked
+
+ macos-test:
+ name: macos test
+ runs-on: macos-latest
+ steps:
+ - uses: actions/checkout@v4
+ with:
+ submodules: true
- name: Setup Rust toolchain
- run: |
- rustup toolchain install ${{ matrix.rust }}
- rustup default ${{ matrix.rust }}
- rustup component add rustfmt
+ uses: ./.github/actions/setup-macos-builder
- name: Run tests
shell: bash
run: |
export PATH=$PATH:$HOME/d/protoc/bin
export ARROW_TEST_DATA=$(pwd)/testing/data
export PARQUET_TEST_DATA=$(pwd)/parquet-testing/data
- export TMP=$(pwd)/.temp
- mkdir -p "$TMP"
- cargo test
- env:
- # do not produce debug symbols to keep memory usage down
- RUSTFLAGS: "-C debuginfo=0"
+ cargo test --profile ci --locked
# verify that the benchmark queries return the correct results
verify-benchmark-results:
name: verify benchmark results (amd64)
- needs: [linux-build-lib]
runs-on: ubuntu-latest
- strategy:
- matrix:
- arch: [amd64]
- rust: [stable]
container:
- image: ${{ matrix.arch }}/rust
- env:
- # Disable full debug symbol generation to speed up CI build and keep memory down
- # "1" means line tables only, which is useful for panic tracebacks.
- RUSTFLAGS: "-C debuginfo=1"
+ image: amd64/rust
steps:
- uses: actions/checkout@v4
with:
submodules: true
- - name: Install protobuf compiler
- shell: bash
- run: |
- apt-get -qq update && apt-get -y -qq install protobuf-compiler
- protoc --version
- - name: Cache Cargo
- uses: actions/cache@v4
- with:
- path: /github/home/.cargo
- # this key equals the ones on `linux-build-lib` for re-use
- key: cargo-cache-
- - name: Cache Rust dependencies
Review Comment:
I also wonder if benchmark test is where we should leave the caching. This is the bottleneck of rust workflows, so any optimization would help.
--
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] improve rust workflows without cache [datafusion-ballista]
Posted by "milenkovicm (via GitHub)" <gi...@apache.org>.
milenkovicm commented on PR #1275:
URL: https://github.com/apache/datafusion-ballista/pull/1275#issuecomment-3094397765
Thanks @Huy1Ng!
As python binding has its problems I wonder if we should keep just basic python CI active
--
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] improve rust workflows without cache [datafusion-ballista]
Posted by "Huy1Ng (via GitHub)" <gi...@apache.org>.
Huy1Ng commented on code in PR #1275:
URL: https://github.com/apache/datafusion-ballista/pull/1275#discussion_r2217684707
##########
.github/actions/setup-builder/action.yaml:
##########
@@ -37,3 +37,5 @@ runs:
rustup toolchain install ${{ inputs.rust-version }}
Review Comment:
I tried but the current repo settings doesn't allow `dtolnay/rust-toolchain@stable`. `datafusion` repo seems to have the same limitations: https://github.com/apache/datafusion/pull/15315.
P/s: my bad. I was using different toolchain. Still, I think we should mirror datafusion CI workflows as much as possible (offload the thinking and decision to the bigger community).
--
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] improve rust workflows without cache [datafusion-ballista]
Posted by "Huy1Ng (via GitHub)" <gi...@apache.org>.
Huy1Ng commented on code in PR #1275:
URL: https://github.com/apache/datafusion-ballista/pull/1275#discussion_r2217685871
##########
.github/workflows/rust.yml:
##########
@@ -17,275 +17,151 @@
name: Rust
+concurrency:
+ group: ${{ github.repository }}-${{ github.workflow }}
+ cancel-in-progress: true
+
on:
# always trigger
push:
pull_request:
jobs:
- # build the library, a compilation step used by multiple steps below
+ # Check crate compiles and base cargo check passes
linux-build-lib:
- name: Build Libraries on AMD64 Rust ${{ matrix.rust }}
+ name: linux build test
runs-on: ubuntu-latest
- strategy:
- matrix:
- arch: [amd64]
- rust: [stable]
container:
- image: ${{ matrix.arch }}/rust
- env:
- # Disable full debug symbol generation to speed up CI build and keep memory down
- # "1" means line tables only, which is useful for panic tracebacks.
- RUSTFLAGS: "-C debuginfo=1"
+ image: amd64/rust
steps:
- uses: actions/checkout@v4
- - name: Cache Cargo
- uses: actions/cache@v4
- with:
- # these represent dependencies downloaded by cargo
- # and thus do not depend on the OS, arch nor rust version.
- path: /github/home/.cargo
- key: cargo-cache-
- - name: Cache Rust dependencies
- uses: actions/cache@v4
with:
- # these represent compiled steps of both dependencies and arrow
- # and thus are specific for a particular OS, arch and rust version.
- path: /github/home/target
- key: ${{ runner.os }}-${{ matrix.arch }}-target-cache-${{ matrix.rust }}-
+ submodules: true
- name: Setup Rust toolchain
uses: ./.github/actions/setup-builder
- with:
- rust-version: ${{ matrix.rust }}
- - name: Build workspace in release mode
+ - name: Prepare cargo build
run: |
- cargo build --release
- ls -l /github/home/target/release
- env:
- CARGO_HOME: "/github/home/.cargo"
- CARGO_TARGET_DIR: "/github/home/target"
- - name: Save artifacts
- uses: actions/upload-artifact@v4
- with:
- name: rust-artifacts
- path: |
- /github/home/target/release/ballista-scheduler
- /github/home/target/release/ballista-executor
+ # Adding `--locked` here to assert that the `Cargo.lock` file is up to
+ # date with the manifest. When this fails, please make sure to commit
+ # the changes to `Cargo.lock` after building with the updated manifest.
+ cargo check --profile ci --workspace --all-targets --locked
- # test the crate
linux-test:
- name: Test Workspace on AMD64 Rust ${{ matrix.rust }}
- needs: [linux-build-lib]
+ name: test linux crates
runs-on: ubuntu-latest
- strategy:
- matrix:
- arch: [amd64]
- rust: [stable]
container:
- image: ${{ matrix.arch }}/rust
- env:
- # Disable full debug symbol generation to speed up CI build and keep memory down
- # "1" means line tables only, which is useful for panic tracebacks.
- RUSTFLAGS: "-C debuginfo=1"
+ image: amd64/rust
steps:
- uses: actions/checkout@v4
with:
submodules: true
- - name: Install protobuf compiler
- shell: bash
- run: |
- apt-get -qq update && apt-get -y -qq install protobuf-compiler
- protoc --version
- - name: Cache Cargo
- uses: actions/cache@v4
- with:
- path: /github/home/.cargo
- # this key equals the ones on `linux-build-lib` for re-use
- key: cargo-cache-
- - name: Cache Rust dependencies
- uses: actions/cache@v4
- with:
- path: /github/home/target
- # this key equals the ones on `linux-build-lib` for re-use
- key: ${{ runner.os }}-${{ matrix.arch }}-target-cache-${{ matrix.rust }}
- name: Setup Rust toolchain
uses: ./.github/actions/setup-builder
- with:
- rust-version: ${{ matrix.rust }}
- name: Run tests
run: |
export PATH=$PATH:$HOME/d/protoc/bin
export ARROW_TEST_DATA=$(pwd)/testing/data
export PARQUET_TEST_DATA=$(pwd)/parquet-testing/data
- cargo test --features=testcontainers
+ cargo test --profile ci --features=testcontainers
env:
CARGO_HOME: "/github/home/.cargo"
CARGO_TARGET_DIR: "/github/home/target"
+
+ linux-build-workspace:
+ name: check linux workspace
+ runs-on: ubuntu-latest
+ container:
+ image: amd64/rust
+ steps:
+ - uses: actions/checkout@v4
+ with:
+ submodules: true
+ - name: Setup Rust toolchain
+ uses: ./.github/actions/setup-builder
- name: Try to compile when `--no-default-features` is selected
run: |
export PATH=$PATH:$HOME/d/protoc/bin
export ARROW_TEST_DATA=$(pwd)/testing/data
export PARQUET_TEST_DATA=$(pwd)/parquet-testing/data
- cargo build -p ballista-scheduler -p ballista-executor -p ballista-core -p ballista --no-default-features
+ cargo check --profile ci -p ballista-scheduler -p ballista-executor -p ballista-core -p ballista --no-default-features --locked
env:
CARGO_HOME: "/github/home/.cargo"
CARGO_TARGET_DIR: "/github/home/target"
- # run ballista tests
ballista-test:
- name: Test Ballista on AMD64 Rust ${{ matrix.rust }}
- needs: [linux-build-lib]
+ name: test linux balista
runs-on: ubuntu-latest
- strategy:
- matrix:
- arch: [amd64]
- rust: [stable]
container:
- image: ${{ matrix.arch }}/rust
- env:
- # Disable full debug symbol generation to speed up CI build and keep memory down
- # "1" means line tables only, which is useful for panic tracebacks.
- RUSTFLAGS: "-C debuginfo=1"
+ image: amd64/rust
steps:
- uses: actions/checkout@v4
with:
submodules: true
- - name: Install protobuf compiler
- shell: bash
- run: |
- mkdir -p $HOME/d/protoc
- cd $HOME/d/protoc
- export PROTO_ZIP="protoc-21.4-linux-x86_64.zip"
- curl -LO https://github.com/protocolbuffers/protobuf/releases/download/v21.4/$PROTO_ZIP
- unzip $PROTO_ZIP
- export PATH=$PATH:$HOME/d/protoc/bin
- protoc --version
- - name: Cache Cargo
- uses: actions/cache@v4
- with:
- path: /github/home/.cargo
- # this key equals the ones on `linux-build-lib` for re-use
- key: cargo-cache-
- - name: Cache Rust dependencies
- uses: actions/cache@v4
- with:
- path: /github/home/target
- # this key equals the ones on `linux-build-lib` for re-use
- key: ${{ runner.os }}-${{ matrix.arch }}-target-cache-${{ matrix.rust }}
- name: Setup Rust toolchain
uses: ./.github/actions/setup-builder
- with:
- rust-version: ${{ matrix.rust }}
- # Ballista is currently not part of the main workspace so requires a separate test step
- name: Run Ballista tests
run: |
export PATH=$PATH:$HOME/d/protoc/bin
export ARROW_TEST_DATA=$(pwd)/testing/data
export PARQUET_TEST_DATA=$(pwd)/parquet-testing/data
cd ballista
# Ensure also compiles in standalone mode
- cargo test --no-default-features --features standalone
+ cargo test --profile ci --no-default-features --features standalone --locked
env:
CARGO_HOME: "/github/home/.cargo"
CARGO_TARGET_DIR: "/github/home/target"
- windows-and-macos:
- name: Test on ${{ matrix.os }} Rust ${{ matrix.rust }}
- runs-on: ${{ matrix.os }}
- strategy:
- matrix:
- os: [windows-latest, macos-latest]
- rust: [stable]
+ windows-test:
+ name: windows test
+ runs-on: windows-latest
steps:
- uses: actions/checkout@v4
with:
submodules: true
- - name: Install protobuf macos compiler
- shell: bash
- run: |
- mkdir -p $HOME/d/protoc
- cd $HOME/d/protoc
- export PROTO_ZIP="protoc-21.4-osx-x86_64.zip"
- curl -LO https://github.com/protocolbuffers/protobuf/releases/download/v21.4/$PROTO_ZIP
- unzip $PROTO_ZIP
- echo "$HOME/d/protoc/bin" >> $GITHUB_PATH
- export PATH=$PATH:$HOME/d/protoc/bin
- protoc --version
- if: ${{matrix.os == 'macos-latest'}}
- - name: Install protobuf windows compiler
+ - name: Setup Rust toolchain
+ uses: ./.github/actions/setup-windows-builder
+ - name: Run tests
shell: bash
run: |
- mkdir -p $HOME/d/protoc
- cd $HOME/d/protoc
- export PROTO_ZIP="protoc-21.4-win64.zip"
- curl -LO https://github.com/protocolbuffers/protobuf/releases/download/v21.4/$PROTO_ZIP
- unzip $PROTO_ZIP
export PATH=$PATH:$HOME/d/protoc/bin
- protoc.exe --version
- if: ${{matrix.os == 'windows-latest'}}
- # TODO: this won't cache anything, which is expensive. Setup this action
- # with a OS-dependent path.
+ export ARROW_TEST_DATA=$(pwd)/testing/data
+ export PARQUET_TEST_DATA=$(pwd)/parquet-testing/data
+ export TMP=$(pwd)/.temp
+ mkdir -p "$TMP"
+ cargo test --profile ci --locked
+
+ macos-test:
+ name: macos test
+ runs-on: macos-latest
+ steps:
+ - uses: actions/checkout@v4
+ with:
+ submodules: true
- name: Setup Rust toolchain
- run: |
- rustup toolchain install ${{ matrix.rust }}
- rustup default ${{ matrix.rust }}
- rustup component add rustfmt
+ uses: ./.github/actions/setup-macos-builder
- name: Run tests
shell: bash
run: |
export PATH=$PATH:$HOME/d/protoc/bin
export ARROW_TEST_DATA=$(pwd)/testing/data
export PARQUET_TEST_DATA=$(pwd)/parquet-testing/data
- export TMP=$(pwd)/.temp
- mkdir -p "$TMP"
- cargo test
- env:
- # do not produce debug symbols to keep memory usage down
- RUSTFLAGS: "-C debuginfo=0"
+ cargo test --profile ci --locked
# verify that the benchmark queries return the correct results
verify-benchmark-results:
name: verify benchmark results (amd64)
- needs: [linux-build-lib]
runs-on: ubuntu-latest
- strategy:
- matrix:
- arch: [amd64]
- rust: [stable]
container:
- image: ${{ matrix.arch }}/rust
- env:
- # Disable full debug symbol generation to speed up CI build and keep memory down
- # "1" means line tables only, which is useful for panic tracebacks.
- RUSTFLAGS: "-C debuginfo=1"
+ image: amd64/rust
steps:
- uses: actions/checkout@v4
with:
submodules: true
- - name: Install protobuf compiler
- shell: bash
- run: |
- apt-get -qq update && apt-get -y -qq install protobuf-compiler
- protoc --version
- - name: Cache Cargo
- uses: actions/cache@v4
- with:
- path: /github/home/.cargo
- # this key equals the ones on `linux-build-lib` for re-use
- key: cargo-cache-
- - name: Cache Rust dependencies
Review Comment:
I will remove them. Those are fast to run so cache wouldn't do much anyway.
--
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] improve rust workflows without cache [datafusion-ballista]
Posted by "milenkovicm (via GitHub)" <gi...@apache.org>.
milenkovicm commented on PR #1275:
URL: https://github.com/apache/datafusion-ballista/pull/1275#issuecomment-3083345764
You don't have to if you think this is better solution. I don't know much about this topic. What do you think ?
I guess we need to publish docker, cancel job is your call
--
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] improve rust workflows without cache [datafusion-ballista]
Posted by "Huy1Ng (via GitHub)" <gi...@apache.org>.
Huy1Ng commented on PR #1275:
URL: https://github.com/apache/datafusion-ballista/pull/1275#issuecomment-3082356061
@milenkovicm can you rerun the CI? I pushed 2 commits too close together so some jobs failed because they didn't get an available machine. The CI run on my fork succeeded: https://github.com/Huy1Ng/datafusion-ballista/tree/improve-github-actions/rust-workflows
--
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] improve rust workflows without cache [datafusion-ballista]
Posted by "milenkovicm (via GitHub)" <gi...@apache.org>.
milenkovicm commented on PR #1275:
URL: https://github.com/apache/datafusion-ballista/pull/1275#issuecomment-3082666517
Have no access to computer at the moment. Can you please push change, it should trigger new job
Two questions
- What's the reason to remove cancel job? It was helpful killing old runs
- We do push docker, that job is used, would it be possible to revert back that change
--
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] improve rust workflows without cache [datafusion-ballista]
Posted by "milenkovicm (via GitHub)" <gi...@apache.org>.
milenkovicm merged PR #1275:
URL: https://github.com/apache/datafusion-ballista/pull/1275
--
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] improve rust workflows without cache [datafusion-ballista]
Posted by "milenkovicm (via GitHub)" <gi...@apache.org>.
milenkovicm commented on code in PR #1275:
URL: https://github.com/apache/datafusion-ballista/pull/1275#discussion_r2217581446
##########
.github/actions/setup-builder/action.yaml:
##########
@@ -37,3 +37,5 @@ runs:
rustup toolchain install ${{ inputs.rust-version }}
Review Comment:
I wonder should we use GitHub action such as `dtolnay/rust-toolchain@stable` to setup rather than use bash script for it?
##########
.github/workflows/rust.yml:
##########
@@ -17,275 +17,151 @@
name: Rust
+concurrency:
+ group: ${{ github.repository }}-${{ github.workflow }}
+ cancel-in-progress: true
+
on:
# always trigger
push:
pull_request:
jobs:
- # build the library, a compilation step used by multiple steps below
+ # Check crate compiles and base cargo check passes
linux-build-lib:
- name: Build Libraries on AMD64 Rust ${{ matrix.rust }}
+ name: linux build test
runs-on: ubuntu-latest
- strategy:
- matrix:
- arch: [amd64]
- rust: [stable]
container:
- image: ${{ matrix.arch }}/rust
- env:
- # Disable full debug symbol generation to speed up CI build and keep memory down
- # "1" means line tables only, which is useful for panic tracebacks.
- RUSTFLAGS: "-C debuginfo=1"
+ image: amd64/rust
steps:
- uses: actions/checkout@v4
- - name: Cache Cargo
- uses: actions/cache@v4
- with:
- # these represent dependencies downloaded by cargo
- # and thus do not depend on the OS, arch nor rust version.
- path: /github/home/.cargo
- key: cargo-cache-
- - name: Cache Rust dependencies
- uses: actions/cache@v4
with:
- # these represent compiled steps of both dependencies and arrow
- # and thus are specific for a particular OS, arch and rust version.
- path: /github/home/target
- key: ${{ runner.os }}-${{ matrix.arch }}-target-cache-${{ matrix.rust }}-
+ submodules: true
- name: Setup Rust toolchain
uses: ./.github/actions/setup-builder
- with:
- rust-version: ${{ matrix.rust }}
- - name: Build workspace in release mode
+ - name: Prepare cargo build
run: |
- cargo build --release
- ls -l /github/home/target/release
- env:
- CARGO_HOME: "/github/home/.cargo"
- CARGO_TARGET_DIR: "/github/home/target"
- - name: Save artifacts
- uses: actions/upload-artifact@v4
- with:
- name: rust-artifacts
- path: |
- /github/home/target/release/ballista-scheduler
- /github/home/target/release/ballista-executor
+ # Adding `--locked` here to assert that the `Cargo.lock` file is up to
+ # date with the manifest. When this fails, please make sure to commit
+ # the changes to `Cargo.lock` after building with the updated manifest.
+ cargo check --profile ci --workspace --all-targets --locked
- # test the crate
linux-test:
- name: Test Workspace on AMD64 Rust ${{ matrix.rust }}
- needs: [linux-build-lib]
+ name: test linux crates
runs-on: ubuntu-latest
- strategy:
- matrix:
- arch: [amd64]
- rust: [stable]
container:
- image: ${{ matrix.arch }}/rust
- env:
- # Disable full debug symbol generation to speed up CI build and keep memory down
- # "1" means line tables only, which is useful for panic tracebacks.
- RUSTFLAGS: "-C debuginfo=1"
+ image: amd64/rust
steps:
- uses: actions/checkout@v4
with:
submodules: true
- - name: Install protobuf compiler
- shell: bash
- run: |
- apt-get -qq update && apt-get -y -qq install protobuf-compiler
- protoc --version
- - name: Cache Cargo
- uses: actions/cache@v4
- with:
- path: /github/home/.cargo
- # this key equals the ones on `linux-build-lib` for re-use
- key: cargo-cache-
- - name: Cache Rust dependencies
- uses: actions/cache@v4
- with:
- path: /github/home/target
- # this key equals the ones on `linux-build-lib` for re-use
- key: ${{ runner.os }}-${{ matrix.arch }}-target-cache-${{ matrix.rust }}
- name: Setup Rust toolchain
uses: ./.github/actions/setup-builder
- with:
- rust-version: ${{ matrix.rust }}
- name: Run tests
run: |
export PATH=$PATH:$HOME/d/protoc/bin
export ARROW_TEST_DATA=$(pwd)/testing/data
export PARQUET_TEST_DATA=$(pwd)/parquet-testing/data
- cargo test --features=testcontainers
+ cargo test --profile ci --features=testcontainers
env:
CARGO_HOME: "/github/home/.cargo"
CARGO_TARGET_DIR: "/github/home/target"
+
+ linux-build-workspace:
+ name: check linux workspace
+ runs-on: ubuntu-latest
+ container:
+ image: amd64/rust
+ steps:
+ - uses: actions/checkout@v4
+ with:
+ submodules: true
+ - name: Setup Rust toolchain
+ uses: ./.github/actions/setup-builder
- name: Try to compile when `--no-default-features` is selected
run: |
export PATH=$PATH:$HOME/d/protoc/bin
export ARROW_TEST_DATA=$(pwd)/testing/data
export PARQUET_TEST_DATA=$(pwd)/parquet-testing/data
- cargo build -p ballista-scheduler -p ballista-executor -p ballista-core -p ballista --no-default-features
+ cargo check --profile ci -p ballista-scheduler -p ballista-executor -p ballista-core -p ballista --no-default-features --locked
env:
CARGO_HOME: "/github/home/.cargo"
CARGO_TARGET_DIR: "/github/home/target"
- # run ballista tests
ballista-test:
- name: Test Ballista on AMD64 Rust ${{ matrix.rust }}
- needs: [linux-build-lib]
+ name: test linux balista
runs-on: ubuntu-latest
- strategy:
- matrix:
- arch: [amd64]
- rust: [stable]
container:
- image: ${{ matrix.arch }}/rust
- env:
- # Disable full debug symbol generation to speed up CI build and keep memory down
- # "1" means line tables only, which is useful for panic tracebacks.
- RUSTFLAGS: "-C debuginfo=1"
+ image: amd64/rust
steps:
- uses: actions/checkout@v4
with:
submodules: true
- - name: Install protobuf compiler
- shell: bash
- run: |
- mkdir -p $HOME/d/protoc
- cd $HOME/d/protoc
- export PROTO_ZIP="protoc-21.4-linux-x86_64.zip"
- curl -LO https://github.com/protocolbuffers/protobuf/releases/download/v21.4/$PROTO_ZIP
- unzip $PROTO_ZIP
- export PATH=$PATH:$HOME/d/protoc/bin
- protoc --version
- - name: Cache Cargo
- uses: actions/cache@v4
- with:
- path: /github/home/.cargo
- # this key equals the ones on `linux-build-lib` for re-use
- key: cargo-cache-
- - name: Cache Rust dependencies
- uses: actions/cache@v4
- with:
- path: /github/home/target
- # this key equals the ones on `linux-build-lib` for re-use
- key: ${{ runner.os }}-${{ matrix.arch }}-target-cache-${{ matrix.rust }}
- name: Setup Rust toolchain
uses: ./.github/actions/setup-builder
- with:
- rust-version: ${{ matrix.rust }}
- # Ballista is currently not part of the main workspace so requires a separate test step
- name: Run Ballista tests
run: |
export PATH=$PATH:$HOME/d/protoc/bin
export ARROW_TEST_DATA=$(pwd)/testing/data
export PARQUET_TEST_DATA=$(pwd)/parquet-testing/data
cd ballista
# Ensure also compiles in standalone mode
- cargo test --no-default-features --features standalone
+ cargo test --profile ci --no-default-features --features standalone --locked
env:
CARGO_HOME: "/github/home/.cargo"
CARGO_TARGET_DIR: "/github/home/target"
- windows-and-macos:
- name: Test on ${{ matrix.os }} Rust ${{ matrix.rust }}
- runs-on: ${{ matrix.os }}
- strategy:
- matrix:
- os: [windows-latest, macos-latest]
- rust: [stable]
+ windows-test:
+ name: windows test
+ runs-on: windows-latest
steps:
- uses: actions/checkout@v4
with:
submodules: true
- - name: Install protobuf macos compiler
- shell: bash
- run: |
- mkdir -p $HOME/d/protoc
- cd $HOME/d/protoc
- export PROTO_ZIP="protoc-21.4-osx-x86_64.zip"
- curl -LO https://github.com/protocolbuffers/protobuf/releases/download/v21.4/$PROTO_ZIP
- unzip $PROTO_ZIP
- echo "$HOME/d/protoc/bin" >> $GITHUB_PATH
- export PATH=$PATH:$HOME/d/protoc/bin
- protoc --version
- if: ${{matrix.os == 'macos-latest'}}
- - name: Install protobuf windows compiler
+ - name: Setup Rust toolchain
+ uses: ./.github/actions/setup-windows-builder
+ - name: Run tests
shell: bash
run: |
- mkdir -p $HOME/d/protoc
- cd $HOME/d/protoc
- export PROTO_ZIP="protoc-21.4-win64.zip"
- curl -LO https://github.com/protocolbuffers/protobuf/releases/download/v21.4/$PROTO_ZIP
- unzip $PROTO_ZIP
export PATH=$PATH:$HOME/d/protoc/bin
- protoc.exe --version
- if: ${{matrix.os == 'windows-latest'}}
- # TODO: this won't cache anything, which is expensive. Setup this action
- # with a OS-dependent path.
+ export ARROW_TEST_DATA=$(pwd)/testing/data
+ export PARQUET_TEST_DATA=$(pwd)/parquet-testing/data
+ export TMP=$(pwd)/.temp
+ mkdir -p "$TMP"
+ cargo test --profile ci --locked
+
+ macos-test:
+ name: macos test
+ runs-on: macos-latest
+ steps:
+ - uses: actions/checkout@v4
+ with:
+ submodules: true
- name: Setup Rust toolchain
- run: |
- rustup toolchain install ${{ matrix.rust }}
- rustup default ${{ matrix.rust }}
- rustup component add rustfmt
+ uses: ./.github/actions/setup-macos-builder
- name: Run tests
shell: bash
run: |
export PATH=$PATH:$HOME/d/protoc/bin
export ARROW_TEST_DATA=$(pwd)/testing/data
export PARQUET_TEST_DATA=$(pwd)/parquet-testing/data
- export TMP=$(pwd)/.temp
- mkdir -p "$TMP"
- cargo test
- env:
- # do not produce debug symbols to keep memory usage down
- RUSTFLAGS: "-C debuginfo=0"
+ cargo test --profile ci --locked
# verify that the benchmark queries return the correct results
verify-benchmark-results:
name: verify benchmark results (amd64)
- needs: [linux-build-lib]
runs-on: ubuntu-latest
- strategy:
- matrix:
- arch: [amd64]
- rust: [stable]
container:
- image: ${{ matrix.arch }}/rust
- env:
- # Disable full debug symbol generation to speed up CI build and keep memory down
- # "1" means line tables only, which is useful for panic tracebacks.
- RUSTFLAGS: "-C debuginfo=1"
+ image: amd64/rust
steps:
- uses: actions/checkout@v4
with:
submodules: true
- - name: Install protobuf compiler
- shell: bash
- run: |
- apt-get -qq update && apt-get -y -qq install protobuf-compiler
- protoc --version
- - name: Cache Cargo
- uses: actions/cache@v4
- with:
- path: /github/home/.cargo
- # this key equals the ones on `linux-build-lib` for re-use
- key: cargo-cache-
- - name: Cache Rust dependencies
Review Comment:
I'm fine to remove cache is it helps, there are another two locations where cache is used `cargo-toml-formatting-checks` and `Clippy`. Would it make sense to remove it there as well or we keep it?
##########
.github/workflows/rust.yml:
##########
@@ -17,275 +17,151 @@
name: Rust
+concurrency:
+ group: ${{ github.repository }}-${{ github.workflow }}
+ cancel-in-progress: true
+
on:
# always trigger
push:
pull_request:
jobs:
- # build the library, a compilation step used by multiple steps below
+ # Check crate compiles and base cargo check passes
linux-build-lib:
- name: Build Libraries on AMD64 Rust ${{ matrix.rust }}
+ name: linux build test
runs-on: ubuntu-latest
- strategy:
- matrix:
- arch: [amd64]
- rust: [stable]
container:
- image: ${{ matrix.arch }}/rust
- env:
- # Disable full debug symbol generation to speed up CI build and keep memory down
- # "1" means line tables only, which is useful for panic tracebacks.
- RUSTFLAGS: "-C debuginfo=1"
+ image: amd64/rust
steps:
- uses: actions/checkout@v4
- - name: Cache Cargo
- uses: actions/cache@v4
- with:
- # these represent dependencies downloaded by cargo
- # and thus do not depend on the OS, arch nor rust version.
- path: /github/home/.cargo
- key: cargo-cache-
- - name: Cache Rust dependencies
- uses: actions/cache@v4
with:
- # these represent compiled steps of both dependencies and arrow
- # and thus are specific for a particular OS, arch and rust version.
- path: /github/home/target
- key: ${{ runner.os }}-${{ matrix.arch }}-target-cache-${{ matrix.rust }}-
+ submodules: true
- name: Setup Rust toolchain
uses: ./.github/actions/setup-builder
- with:
- rust-version: ${{ matrix.rust }}
- - name: Build workspace in release mode
+ - name: Prepare cargo build
run: |
- cargo build --release
- ls -l /github/home/target/release
- env:
- CARGO_HOME: "/github/home/.cargo"
- CARGO_TARGET_DIR: "/github/home/target"
- - name: Save artifacts
- uses: actions/upload-artifact@v4
- with:
- name: rust-artifacts
- path: |
- /github/home/target/release/ballista-scheduler
- /github/home/target/release/ballista-executor
+ # Adding `--locked` here to assert that the `Cargo.lock` file is up to
Review Comment:
👍🏻
##########
.github/workflows/rust.yml:
##########
@@ -17,275 +17,151 @@
name: Rust
+concurrency:
+ group: ${{ github.repository }}-${{ github.workflow }}
+ cancel-in-progress: true
Review Comment:
👍🏻
--
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] improve rust workflows without cache [datafusion-ballista]
Posted by "Huy1Ng (via GitHub)" <gi...@apache.org>.
Huy1Ng commented on PR #1275:
URL: https://github.com/apache/datafusion-ballista/pull/1275#issuecomment-3094376920
this should be ready for rust. I will tackle Python workflow next. This is going to be spitted into 3 small PRs.
--
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