Skip to content

chore(ci): Register the substrait check job with runs-on, route spark checks through xtask - #25195

Open
namanjain24-sudo wants to merge 1 commit into
apache:mainfrom
namanjain24-sudo:ci-align-check-jobs
Open

chore(ci): Register the substrait check job with runs-on, route spark checks through xtask#25195
namanjain24-sudo wants to merge 1 commit into
apache:mainfrom
namanjain24-sudo:ci-align-check-jobs

Conversation

@namanjain24-sudo

Copy link
Copy Markdown

Which issue does this PR close?

Rationale for this change

While measuring the cargo check <crate> features jobs for #25148 I found two inconsistencies in them. Neither is a policy question about which runner to use, so they seemed worth separating out.

1. The substrait check job never registers with runs-on.

Fifteen jobs in rust.yml select a runs-on runner with extras=s3-cache. Fourteen of them start with the runs-on/action step that registers the job. cargo check datafusion-substrait features is the one that does not, so on push and merge_group it lands on the runs-on runner but gets neither the external cache nor the job metrics the other fourteen report. It looks like an oversight rather than a choice; the job directly above and below it in the file both have the step.

2. The spark check job is the last one not going through xtask.

cargo check datafusion-spark features calls cargo check directly. Every other check step in the workflow goes through cargo xtask ci step check, which is what lets a contributor run the same thing locally. After this PR there is no raw cargo check left in rust.yml.

What changes are included in this PR?

Four lines.

  • Add the runs-on/action step to linux-datafusion-substrait-features, matching its siblings.
  • Replace the three datafusion-spark cargo check invocations with the xtask equivalents.

The commands xtask emits are the same ones being replaced, which --explain confirms:

step before cargo xtask ci step check datafusion-spark …
default features cargo check --profile ci --all-targets -p datafusion-spark default
no-default-features cargo check --profile ci --no-default-features -p datafusion-spark no-default
core cargo check --profile ci --no-default-features -p datafusion-spark --features=core core

I deliberately left out the larger changes those jobs invite, because they are the open question in #25148 rather than something to decide in a cleanup PR. For the record, what I measured there: five of the seven cargo check <crate> features jobs declare no Rust Dependency Cache step at all, only datafusion and datafusion-substrait do; three of the seven are pinned to ubuntu-latest rather than the vars.USE_RUNS_ON expression; and across those seven jobs about a third of the wall clock is setup and cache restore paid seven times over. Happy to follow up on any of that if it is wanted.

What is the testing strategy for this PR?

The three xtask commands were run locally against this branch and all pass:

cargo xtask ci step check datafusion-spark default
cargo xtask ci step check datafusion-spark no-default
cargo xtask ci step check datafusion-spark core

python3 ci/scripts/check_asf_yaml_status_checks.py reports OK: All 32 required_status_checks match existing GitHub Actions jobs — job names are unchanged, so the required checks are unaffected. ci/scripts/check_no_cargo_install_in_workflows.sh and ci/scripts/typos_check.sh are clean, and the file still parses as YAML with the same 26 jobs.

The runs-on/action half cannot be verified outside the Apache repo, since the action is a no-op on standard GitHub runners.

Are there any user-facing changes?

No. CI configuration only.

… checks through xtask

Two small inconsistencies in the `cargo check <crate> features` jobs.

`cargo check datafusion-substrait features` selects a `runs-on` runner with
`extras=s3-cache`, but is the only one of the fifteen jobs that select such a
runner without the `runs-on/action` step that registers it. Without that step
the job gets neither the external cache nor the job metrics the other fourteen
report.

`cargo check datafusion-spark features` is the last place in the workflow that
calls `cargo check` directly rather than going through `cargo xtask ci step
check`, so those three checks cannot be reproduced locally the way every other
check step can.

The three commands xtask emits are identical to the ones being replaced:

  default    -> cargo check --profile ci --all-targets -p datafusion-spark
  no-default -> cargo check --profile ci --no-default-features -p datafusion-spark
  core       -> cargo check --profile ci --no-default-features -p datafusion-spark --features core

Job names are unchanged, so the required checks in `.asf.yaml` are unaffected.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

development-process Related to development process of DataFusion

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant