ci: run the Rust workflow for fork pull requests on the fork by kumarUjjawal · Pull Request #25591 · apache/datafusion

kumarUjjawal · 2026-09-22T05:15:22Z

Which issue does this PR close?

Part of #25148.

Rationale for this change

Fork pull requests run the full Rust workflow upstream. Many contributors also run that workflow on their forks, which adds demand on shared ASF runners.

This PR uses fork runs for feedback on pull requests to main. The fork run tests the branch head. The merge queue still runs all 27 Rust jobs on the merged result before merge.

What changes are included in this PR?

Upstream skips 24 Rust jobs for fork pull requests to main. check-fmt, check-workflow-tool-installs, and examples-check-datafusion-crates still run.
A Rust (fork) check shows the fork run for the pull request's head commit. Scheduled updates pick up late runs and reruns.
Pull requests from upstream branches and fork pull requests to release branches keep the full Rust workflow.
CI and local lints validate the skip conditions, job dependencies, and artifact consumers.

The bridge uses pull_request_target and the GitHub API. It never checks out or runs pull request code. The fork check is informational because contributors control their fork's workflows. It is not a required check for merge.

What is the testing strategy for this PR?

Local checks passed for the job conditions, dependencies, and artifact guards. Bridge probes with stubbed API responses covered missing runs, late runs, reruns, and pagination beyond 500 open pull requests.

Hosted end-to-end validation of the new bridge remains outstanding. pull_request_target uses the base branch's workflow, so this pull request cannot exercise the new bridge on GitHub.

Are there any user-facing changes?

Contributors must enable GitHub Actions on their forks. The contributor guide and PR template explain this requirement. Pushes that change only documentation or Markdown do not start the Rust workflow.

A pull request from a fork already runs rust.yml on the fork. Upstream ran the same 26 jobs again on GitHub-hosted runners, because the vars context is empty for a fork and every job falls back to ubuntu-latest. Skip 24 jobs upstream for a fork pull request to main and keep the two fast lint jobs. Add fork_ci_status.yml, which mirrors the fork run as one `Rust (fork)` check on the pull request without checking out or running pull request code. The merge queue still runs every job on the merged result. Fork pull requests to release branches, which have no merge queue, keep the full run. Move the skip-condition validation into check_rust_workflow_skips.py and return check_asf_yaml_status_checks.py to its original scope. Part of apache#25148

kumarUjjawal · 2026-09-22T05:20:53Z

@alamb @comphead PTAL

kumarUjjawal · 2026-09-22T05:21:03Z

Fork's run: https://github.com/kumarUjjawal/datafusion/actions/runs/35689993933

kosiew

@kumarUjjawal,

Thanks for working on this. The approach looks good, and I don't see any blocking issues. I have a few non-blocking suggestions to make the fork CI status more resilient and strengthen the workflow validation.

kosiew · 2026-10-14T06:10:07Z

+              const fork = { owner: params.owner, repo: params.repo };
+              const sha = params.head_sha || pr.head.sha;
+              let run = null;
+              if (params.run_id) {


Could we look for a replacement run when the recorded run is cancelled, fails, or can no longer be fetched? A workflow_dispatch run can cancel the push run for the same SHA, leaving the check showing the old result. We should select the newest run across event types rather than always preferring push runs.

kosiew · 2026-10-14T06:10:07Z

+                per_page: 5,
+              });
+              const check = data.check_runs[0];
+              if (!check || !check.output || !check.output.text) return;


If the initial notification fails before creating the check, the scheduled sweep cannot recover it. Could we let the sweep create a missing check using pr.head, or document that maintainers need to rerun the bridge job?

kosiew · 2026-10-14T06:10:07Z

+    return [needs] if isinstance(needs, str) else list(needs)
+
+
+def check_skip_conditions(workflow, required_checks):


Could we add a few negative tests to confirm this validator catches regressions? For example, remove a skip guard, make an unskipped job depend on a skipped job, or remove an artifact-upload guard. This can be a follow-up.

kumarUjjawal · 2026-10-18T10:25:01Z

Thank you @kosiew

Addressed all the comments please take a look.

kosiew

@kumarUjjawal,

Thanks for addressing the review comments. I've reviewed the follow-up changes and confirmed that all three issues have been resolved.

The workflow now selects the newest fork run across event types, recovers missing checks during scheduled sweeps, and includes the requested validator negative tests.

I found no new issues. LGTM!

alamb

Thanks @kumarUjjawal and @kosiew this looks like it will be amazing and I think it will align costs nicely (the ASF won't pay credits to test the ever increasing number of PRs from others)

I just worry this will cause confusion with submitters when the CI doesn't run and they don't get any signal.

Maybe we can somehow post a note to the PR (or put a note in the PR template) that running the CI requires some extra steps

Also, after we merge this I think it would be helpful to post a note to dev@datafusion.apache.org to update people on the flow

alamb · 2026-10-19T10:32:07Z

+1. Enable workflows on your fork. Open the **Actions** tab of your fork and
+   confirm the prompt. If the `Rust (fork)` check reports no fork run for a
+   commit that changed code, this step is missing; enable workflows and push
+   again. A push that changes only documentation or Markdown does not start


A push that changes only documentation or Markdown does not start
the Rust workflow; the check then reports that no fork run exists for the
commit, and the merge queue still runs the full workflow.

That seems like it describes an implementation detail and could probably be removed to make this paragraph easier to read

kumarUjjawal · 2026-10-19T14:21:56Z

Also, after we merge this I think it would be helpful to post a note to dev@datafusion.apache.org to update people on the flow

Sounds good!

kumarUjjawal · 2026-10-19T14:29:24Z

Maybe we can somehow post a note to the PR (or put a note in the PR template) that running the CI requires some extra steps

Added the [!IMPORTANT] tag

alamb

Looks good -- thank you @kumarUjjawal .

Let's give it a try!

comphead · 2026-10-19T21:06:14Z

Thats a good thing, happy it moves into this direction. We had the same experiment in Comet months ago(apache/datafusion-comet#4289) and we should probably pursue it again.

Let me also review it.

comphead · 2026-10-19T21:11:52Z

We also need to check how this would work with Merge Queue

comphead

Thanks @kumarUjjawal, this targets the right workflow. In a sample of 30 completed fork PR runs of rust.yml from Oct 18 and 19, about 136 of 138 job-minutes per run belong to the 24 jobs this PR skips. The other PR-time workflows (breaking_changes_detector, audit, docs_pr) cost about 2 to 3 minutes per run each. Fork runners should not be a slowdown here either, since most upstream PR jobs already use ubuntu-latest. My cargo test (amd64) comparison is one fork run at about 14 minutes against an upstream average of 18 minutes.

Some non-blocking suggestions, with two inline comments on the bridge.

Fork concurrency group. rust.yml groups by github.head_ref || github.sha. A push to a fork branch has no head_ref, so every push gets its own group and superseded fork runs are never cancelled. The bridge only mirrors the head commit, so those runs are wasted on the contributor's account and delay the result. Spark avoids this with github.repository == 'apache/spark' && github.run_id || github.ref. Something like github.head_ref || (github.repository == 'apache/datafusion' && github.sha || github.ref) would keep upstream main and the merge queue per SHA. I have not tested this.
Stale branches. The fork run tests the branch head, not the merge result. A green branch that is behind main can still fail in the queue and evict the PR, which stalls the entries behind it. Two options. The bridge could report neutral with a hint when compareCommits shows the branch is far behind. Or linux-build-lib (about 6 minutes, roughly 4% of a run) could stay on for fork PRs as a merge-ref compile smoke test.
Fork main pushes. A fork's "Sync fork" push to main starts the full workflow on the contributor's runners. Spark skips default-branch pushes on forks. This does not affect ASF minutes, so it is optional.
Skip list. The description says two jobs stay on upstream, but Check datafusion crates used in datafusion-examples (rust.yml, ubuntu-slim, about 0.2 minutes) is not in FORK_PR_SKIPPED_JOBS and ran in my sample. That seems fine, but the description and comment could say three.
Gate. The Rust (fork) check is only as trustworthy as the fork's rust.yml, because the contributor controls it. Keeping it informational with the merge queue as the gate is the right call. It may be worth tracking queue evictions before and after rollout.

comphead · 2026-10-19T21:17:20Z

+jobs:
+  bridge:
+    name: Mirror the fork run
+    runs-on: ubuntu-latest


Could this use ubuntu-slim, as dev.yml and labeler.yml do? In my sample of recent fork PR runs, ubuntu-latest jobs waited about 67 to 88 minutes to start on average, while the one ubuntu-slim job waited about 2 minutes. The numbers are noisy, but if this job sits in the saturated pool, the check can appear an hour late. The script needs no more than the slim runner offers, and timeout-minutes: 10 fits its limit. Spark's notifier and updater both use ubuntu-slim.

comphead · 2026-10-19T21:17:20Z

+              return last ? Number(last[1]) : response.data.length;
+            }
+
+            async function update() {


Every tick calls checks.listForRef and the fork's listWorkflowRuns for each fork PR updated in the last six hours, plus the 25-PR slice. With about 50 active PRs I estimate roughly 200 requests per tick, or about 800 per hour. That is unverified and close to the 1,000 per hour GITHUB_TOKEN limit mentioned above, and the pull_request_target retries add to it. The limit is per repository, so exhausting it would also hit the other workflows. Two ways to cut it. Re-poll only checks that are still queued or in_progress or have no run, and leave completed ones to the slice sweep, which is already how older PRs are handled. Or read all check states in one GraphQL query and call the fork only when needed.

kumarUjjawal · 2026-10-20T13:57:08Z

Thank you @comphead

I have addressed your review items, I’ve left stale-branch handling and queue-eviction tracking for follow-up. Fork main runs remain enabled because they populate caches used by branch runs.

PTAL

neilconway

Thanks @kumarUjjawal! Let's please get this merged soon 😅

neilconway · 2026-10-20T15:03:08Z

+  pull_request_target:
+    types: [opened, reopened, synchronize]


Apparently pull_request_target is going away next month: https://infra.apache.org/github-actions-policy.html

The ASF page suggests a workaround; will that work for what we need? But I'd be fine landing this as-is and then fixing before the end of the month.

ouch, this is a good info

let's merge this for now and we can find a more permanent solution in follow ups

what do you think @comphead @neilconway

but IMO it should be fine, with user forks, the pull_request_target is called on the contributor fork

It looks like there are security implications using pull_request_target -- are we worried about that ?

It looks like there are security implications using pull_request_target -- are we worried about that ?

IMO as long as it runs on user forks(which is), we are fine

comphead

Thanks @kumarUjjawal lets give it a try

kumarUjjawal · 2026-10-20T15:26:20Z

Thank you all!

comphead · 2026-10-20T23:37:03Z

The edge case is to check how dependabot works with user forks

kumarUjjawal · 2026-10-21T02:23:03Z

The edge case is to check how dependabot works with user forks

I will do some investigation and keep an eye on it so we don't miss anything.

github-actions Bot added documentation Improvements or additions to documentation development-process Related to development process of DataFusion labels Sep 22, 2026

kumarUjjawal mentioned this pull request Sep 22, 2026

Reduce GitHub Actions usage (DataFusion is currently the top ASF consumer of GHA minutes) #25148

Open

Merge branch 'main' into kumar/fork-ci

38486f7

kosiew approved these changes Oct 14, 2026

View reviewed changes

ci: recover fork status checks and test workflow guards

713df1f

kosiew approved these changes Oct 19, 2026

View reviewed changes

alamb reviewed Oct 19, 2026

View reviewed changes

docs: clarify fork CI setup and runner usage

7a0f8cf

kumarUjjawal requested a review from alamb October 19, 2026 14:29

alamb approved these changes Oct 19, 2026

View reviewed changes

comphead reviewed Oct 19, 2026

View reviewed changes

ci: reduce fork run and status polling overhead

2ff4d6b

neilconway approved these changes Oct 20, 2026

View reviewed changes

comphead approved these changes Oct 20, 2026

View reviewed changes

kumarUjjawal enabled auto-merge October 20, 2026 15:26

Merge branch 'main' into kumar/fork-ci

0dabf5f

kumarUjjawal added this pull request to the merge queue Oct 20, 2026

alamb mentioned this pull request Oct 20, 2026

Consider running Miri on a scheduled basis apache/arrow-rs#11548

Open

Merged via the queue into apache:main with commit 00c8ecb Oct 20, 2026
43 checks passed

kumarUjjawal deleted the kumar/fork-ci branch October 20, 2026 21:55

github-actions Bot added the v56.0.0 label Oct 20, 2026

kumarUjjawal mentioned this pull request Oct 21, 2026

Discuss bringing datafusion-distributed under Apache DataFusion governance datafusion-contrib/datafusion-distributed#784

Open

neilconway mentioned this pull request Oct 21, 2026

chore: run the circular dependency check on the fork for fork PRs #26574

Open

		return [needs] if isinstance(needs, str) else list(needs)


		def check_skip_conditions(workflow, required_checks):

Conversation

kumarUjjawal commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

What is the testing strategy for this PR?

Are there any user-facing changes?

Uh oh!

kumarUjjawal commented Sep 22, 2026

Uh oh!

kumarUjjawal commented Sep 22, 2026

Uh oh!

kosiew left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kumarUjjawal commented Oct 18, 2026

Uh oh!

kosiew left a comment

Choose a reason for hiding this comment

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kumarUjjawal commented Oct 19, 2026

Uh oh!

kumarUjjawal commented Oct 19, 2026

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

comphead commented Oct 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

comphead commented Oct 19, 2026

Uh oh!

comphead left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kumarUjjawal commented Oct 20, 2026

Uh oh!

neilconway left a comment

Choose a reason for hiding this comment

Uh oh!

neilconway Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

comphead left a comment

Choose a reason for hiding this comment

Uh oh!

kumarUjjawal commented Oct 20, 2026

Uh oh!

Uh oh!

comphead commented Oct 20, 2026

Uh oh!

kumarUjjawal commented Oct 21, 2026

kumarUjjawal commented Sep 22, 2026 •

edited

Loading

comphead commented Oct 19, 2026 •

edited

Loading

neilconway Oct 20, 2026 •

edited

Loading