kosiew commented on code in PR #25591: URL: https://github.com/apache/datafusion/pull/25591#discussion_r4267364307
########## .github/workflows/fork_ci_status.yml: ########## @@ -0,0 +1,320 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# # Fork CI status +# +# A pull request from a fork to `main` runs `rust.yml` on the fork. This +# workflow mirrors that run as one `Rust (fork)` check on the pull request, so +# reviewers see the result without opening the fork. It never checks out or +# runs code from the pull request: on `pull_request_target` it reads the fork's +# public run list and creates the check, and on a schedule it updates the +# checks of recently updated pull requests plus a slice of the older ones. The +# check is informational. The merge queue runs the full workflow on the merged +# result and stays the gate. +# +# See docs/source/contributor-guide/index.md, "Pull Requests from Forks". +name: Fork CI status + +on: + pull_request_target: + types: [opened, reopened, synchronize] + schedule: + - cron: "*/15 * * * *" + workflow_dispatch: + inputs: + hours: + description: "Update every pull request updated within this many hours" + type: number + default: 6 + +permissions: + actions: read + checks: write + pull-requests: read + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || 'update' }} + cancel-in-progress: ${{ github.event_name == 'pull_request_target' }} + +jobs: + bridge: + name: Mirror the fork run + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Create or update the check + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + env: + UPDATE_HOURS: ${{ inputs.hours || 6 }} + with: + script: | + const CHECK = 'Rust (fork)'; + const WORKFLOW = 'rust.yml'; + const upstream = { owner: context.repo.owner, repo: context.repo.repo }; + const GUIDE = 'https://datafusion.apache.org/contributor-guide/#pull-requests-from-forks'; + // Older pull requests are revisited this many at a time, so a rerun + // or a late run on a quiet pull request is mirrored within hours. + const SWEEP_SIZE = 25; + + // A check run accepts a fixed set of conclusions; a workflow run can + // report others, such as startup_failure. + const CONCLUSIONS = new Set([ + 'action_required', 'cancelled', 'failure', 'neutral', + 'skipped', 'stale', 'success', 'timed_out', + ]); + const conclusionOf = (run) => + CONCLUSIONS.has(run.conclusion) ? run.conclusion : 'failure'; + const statusOf = (run) => { + if (run.status === 'completed') return 'completed'; + return run.status === 'in_progress' ? 'in_progress' : 'queued'; + }; + const isForkPrToMain = (pr) => + Boolean(pr.head.repo) && pr.head.repo.fork === true && pr.base.ref === 'main'; + + const HOW_UPDATED = + 'This check updates within 15 minutes for a pull request updated in the last six hours, ' + + 'and within a few hours otherwise. The merge queue runs the full workflow on the merged ' + + 'result before this pull request merges.'; + + // The check for a run. `text` is read back by the scheduled update. + const runPayload = (fork, sha, run) => { + const status = statusOf(run); + return { + status, + ...(status === 'completed' ? { conclusion: conclusionOf(run) } : {}), + details_url: run.html_url, + output: { + title: `Fork run ${status === 'completed' ? run.conclusion : status}`, + summary: [ + `[Rust workflow run on ${fork.owner}/${fork.repo}](${run.html_url}) for this commit.`, + '', + 'If the fork run failed for reasons unrelated to this pull request, rerun it on the fork.', + HOW_UPDATED, + ].join('\n'), + text: JSON.stringify({ + owner: fork.owner, + repo: fork.repo, + head_sha: sha, + run_id: run.id, + run_attempt: run.run_attempt, + }), + }, + }; + }; + + // The check when the fork has no run for the commit. The workflow's + // state on the fork says whether workflows are enabled there; a + // push that changes only documentation or Markdown does not start + // rust.yml even when they are. The scheduled update keeps looking + // for a run in every case. + const missingPayload = (fork, sha, state) => { + const disabled = typeof state === 'string' && state.startsWith('disabled'); + const active = state === 'active'; + const advice = disabled + ? [ + '1. Enable workflows on your fork: open its **Actions** tab and confirm the prompt.', + '2. Push again. A push that changes only documentation or Markdown does not start', + ' the Rust workflow.', + ] + : active + ? [ + 'A push that changes only documentation or Markdown does not start the Rust', + 'workflow, and a run can take a moment to appear. This check keeps looking for one.', + ] + : [ + 'GitHub reports no Rust workflow on the fork, which usually means workflows are not', + 'enabled there. Enable them on the fork\'s **Actions** tab and push again. A push that', + 'changes only documentation or Markdown does not start the Rust workflow.', + ]; + return { + status: 'completed', + conclusion: disabled ? 'action_required' : active ? 'skipped' : 'neutral', + output: { + title: disabled ? 'Workflows are disabled on the fork' : 'No fork run for this commit', + summary: [ + `No Rust workflow run was found on ${fork.owner}/${fork.repo} for ${sha}.`, + '', + ...advice, + '', + HOW_UPDATED, + '', + `See ${GUIDE}`, + ].join('\n'), + text: JSON.stringify({ owner: fork.owner, repo: fork.repo, head_sha: sha, run_id: null }), + }, + }; + }; + + // The fork creates its run a few seconds after the push. + async function findRun(fork, sha, attempts) { + for (let attempt = 0; attempt < attempts; attempt++) { + try { + const { data } = await github.rest.actions.listWorkflowRuns({ + owner: fork.owner, + repo: fork.repo, + workflow_id: WORKFLOW, + head_sha: sha, + per_page: 10, + }); + const run = + data.workflow_runs.find((r) => r.event === 'push') || data.workflow_runs[0]; + if (run) return run; + } catch (error) { + // 404 when the fork has Actions disabled or lacks the workflow. + core.info(`No run list for ${fork.owner}/${fork.repo}: ${error.message}`); + } + if (attempt + 1 < attempts) { + await new Promise((resolve) => setTimeout(resolve, 5000)); + } + } + return null; + } + + // `active`, or `disabled_fork` until the owner enables workflows; + // null when the workflow cannot be read. + async function workflowState(fork) { + try { + const { data } = await github.rest.actions.getWorkflow({ + owner: fork.owner, + repo: fork.repo, + workflow_id: WORKFLOW, + }); + return data.state || null; + } catch (error) { + core.info(`No workflow state for ${fork.owner}/${fork.repo}: ${error.message}`); + return null; + } + } + + async function notify() { + const pr = context.payload.pull_request; + // Only fork PRs to main skip jobs upstream; release branches have + // no merge queue and keep the full run. + if (!isForkPrToMain(pr)) { + core.info('Not a fork pull request to main; nothing to mirror.'); + return; + } + const fork = { owner: pr.head.repo.owner.login, repo: pr.head.repo.name }; + const sha = pr.head.sha; + const run = await findRun(fork, sha, 12); + const payload = run + ? runPayload(fork, sha, run) + : missingPayload(fork, sha, await workflowState(fork)); + await github.rest.checks.create({ ...upstream, name: CHECK, head_sha: sha, ...payload }); + core.info(`Created ${CHECK} for ${sha}: ${payload.conclusion || payload.status}`); + } + + // Bring one pull request's check in line with its fork run. A check + // without a run keeps looking for one; a check with a run picks up + // progress and reruns. + async function syncCheck(pr) { + const { data } = await github.rest.checks.listForRef({ + ...upstream, + ref: pr.head.sha, + check_name: CHECK, + per_page: 5, + }); + const check = data.check_runs[0]; + if (!check || !check.output || !check.output.text) return; + let params; + try { + params = JSON.parse(check.output.text); + } catch { + return; + } + const fork = { owner: params.owner, repo: params.repo }; + const sha = params.head_sha || pr.head.sha; + let run = null; + if (params.run_id) { Review Comment: 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. ########## ci/scripts/check_rust_workflow_skips.py: ########## @@ -0,0 +1,284 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +""" +Validate the job-level skip conditions in rust.yml. + +rust.yml skips jobs on two events. Each skip set below pairs a clause with +the jobs that carry it: a job's `if:` is exactly `!(clause)` for every set +it belongs to, joined with ` && ` in the order of SKIP_POLICIES, and a job +in no set carries no job-level `if:`. A new job therefore runs everywhere +by default, and changing a set is a deliberate edit to both files. + +The Cargo check artifact steps in `linux-build-lib` and the jobs that +download the artifact must follow the same sets. +""" + +import os +import sys + +import yaml + +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) + +from check_asf_yaml_status_checks import get_required_checks # noqa: E402 + +RUST_WORKFLOW = "rust.yml" + +# Skipped on a push to upstream main; see "Pushes to main" in rust.yml. +POST_MERGE_PUSH = ( + "github.event_name == 'push'" + " && github.ref == 'refs/heads/main'" + " && github.repository == 'apache/datafusion'" +) +POST_MERGE_SKIPPED_JOBS = frozenset( + { + "cargo-toml-formatting-checks", + "check-fmt", + "check-workflow-tool-installs", + "config-docs-check", + "examples-docs-check", + "linux-cargo-check-datafusion", + "linux-cargo-check-datafusion-spark", + "linux-datafusion-common-features", + "linux-datafusion-proto-features", + "linux-datafusion-substrait-features", + "linux-rustdoc", + "linux-test-datafusion-cli", + "linux-test-doc", + "linux-wasm-pack", + "msrv", + "sqllogictest-postgres", + "sqllogictest-substrait", + "vendor", + "verify-benchmark-results", + } +) + +# Skipped on a pull request from a fork; see "Pull requests from forks" in +# rust.yml and fork_ci_status.yml. +FORK_PULL_REQUEST = ( + "github.event_name == 'pull_request'" + " && github.event.pull_request.head.repo.fork" + " && github.base_ref == 'main'" +) +FORK_PR_SKIPPED_JOBS = frozenset( + { + "cargo-toml-formatting-checks", + "clippy", + "config-docs-check", + "examples-docs-check", + "linux-build-lib", + "linux-cargo-check-datafusion", + "linux-cargo-check-datafusion-functions", + "linux-cargo-check-datafusion-spark", + "linux-datafusion-common-features", + "linux-datafusion-ffi-features", + "linux-datafusion-proto-features", + "linux-datafusion-substrait-features", + "linux-rustdoc", + "linux-test", + "linux-test-datafusion-cli", + "linux-test-doc", + "linux-test-example", + "linux-wasm-pack", + "macos-aarch64", + "msrv", + "sqllogictest-postgres", + "sqllogictest-substrait", + "vendor", + "verify-benchmark-results", + } +) + +SKIP_POLICIES = ( + ("POST_MERGE_SKIPPED_JOBS", POST_MERGE_SKIPPED_JOBS, POST_MERGE_PUSH), + ("FORK_PR_SKIPPED_JOBS", FORK_PR_SKIPPED_JOBS, FORK_PULL_REQUEST), +) + + +def expected_condition(job): + """The exact job-level `if:` for a job, or None when it must carry none.""" + clauses = [clause for _, jobs, clause in SKIP_POLICIES if job in jobs] + if not clauses: + return None + return "${{ " + " && ".join(f"!({clause})" for clause in clauses) + " }}" + + +# `linux-build-lib` uploads Cargo check artifacts for two consumers. Its steps +# skip on a push to main and the whole job skips on a fork PR. +ARTIFACT_JOB = "linux-build-lib" +ARTIFACT_ARCHIVE_STEP = "archive-check" +ARTIFACT_NAME = "cargo-check" +ARTIFACT_ARCHIVE_CONDITION = "${{ !(" + POST_MERGE_PUSH + ") }}" +ARTIFACT_UPLOAD_CONDITION = ( + "${{ steps." + ARTIFACT_ARCHIVE_STEP + ".outcome == 'success'" + " && !(" + POST_MERGE_PUSH + ") }}" +) + + +def iter_steps(job_config): + """Yield the steps of a job, flattening `parallel:` groups.""" + for step in job_config.get("steps") or []: + if not isinstance(step, dict): + continue + if "parallel" in step: + yield from (s for s in step["parallel"] or [] if isinstance(s, dict)) + else: + yield step + + +def uses_action(step, action): + return str(step.get("uses", "")).startswith(action) + + +def job_needs(config): + needs = config.get("needs") or [] + return [needs] if isinstance(needs, str) else list(needs) + + +def check_skip_conditions(workflow, required_checks): Review Comment: 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. ########## .github/workflows/fork_ci_status.yml: ########## @@ -0,0 +1,320 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# # Fork CI status +# +# A pull request from a fork to `main` runs `rust.yml` on the fork. This +# workflow mirrors that run as one `Rust (fork)` check on the pull request, so +# reviewers see the result without opening the fork. It never checks out or +# runs code from the pull request: on `pull_request_target` it reads the fork's +# public run list and creates the check, and on a schedule it updates the +# checks of recently updated pull requests plus a slice of the older ones. The +# check is informational. The merge queue runs the full workflow on the merged +# result and stays the gate. +# +# See docs/source/contributor-guide/index.md, "Pull Requests from Forks". +name: Fork CI status + +on: + pull_request_target: + types: [opened, reopened, synchronize] + schedule: + - cron: "*/15 * * * *" + workflow_dispatch: + inputs: + hours: + description: "Update every pull request updated within this many hours" + type: number + default: 6 + +permissions: + actions: read + checks: write + pull-requests: read + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || 'update' }} + cancel-in-progress: ${{ github.event_name == 'pull_request_target' }} + +jobs: + bridge: + name: Mirror the fork run + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Create or update the check + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + env: + UPDATE_HOURS: ${{ inputs.hours || 6 }} + with: + script: | + const CHECK = 'Rust (fork)'; + const WORKFLOW = 'rust.yml'; + const upstream = { owner: context.repo.owner, repo: context.repo.repo }; + const GUIDE = 'https://datafusion.apache.org/contributor-guide/#pull-requests-from-forks'; + // Older pull requests are revisited this many at a time, so a rerun + // or a late run on a quiet pull request is mirrored within hours. + const SWEEP_SIZE = 25; + + // A check run accepts a fixed set of conclusions; a workflow run can + // report others, such as startup_failure. + const CONCLUSIONS = new Set([ + 'action_required', 'cancelled', 'failure', 'neutral', + 'skipped', 'stale', 'success', 'timed_out', + ]); + const conclusionOf = (run) => + CONCLUSIONS.has(run.conclusion) ? run.conclusion : 'failure'; + const statusOf = (run) => { + if (run.status === 'completed') return 'completed'; + return run.status === 'in_progress' ? 'in_progress' : 'queued'; + }; + const isForkPrToMain = (pr) => + Boolean(pr.head.repo) && pr.head.repo.fork === true && pr.base.ref === 'main'; + + const HOW_UPDATED = + 'This check updates within 15 minutes for a pull request updated in the last six hours, ' + + 'and within a few hours otherwise. The merge queue runs the full workflow on the merged ' + + 'result before this pull request merges.'; + + // The check for a run. `text` is read back by the scheduled update. + const runPayload = (fork, sha, run) => { + const status = statusOf(run); + return { + status, + ...(status === 'completed' ? { conclusion: conclusionOf(run) } : {}), + details_url: run.html_url, + output: { + title: `Fork run ${status === 'completed' ? run.conclusion : status}`, + summary: [ + `[Rust workflow run on ${fork.owner}/${fork.repo}](${run.html_url}) for this commit.`, + '', + 'If the fork run failed for reasons unrelated to this pull request, rerun it on the fork.', + HOW_UPDATED, + ].join('\n'), + text: JSON.stringify({ + owner: fork.owner, + repo: fork.repo, + head_sha: sha, + run_id: run.id, + run_attempt: run.run_attempt, + }), + }, + }; + }; + + // The check when the fork has no run for the commit. The workflow's + // state on the fork says whether workflows are enabled there; a + // push that changes only documentation or Markdown does not start + // rust.yml even when they are. The scheduled update keeps looking + // for a run in every case. + const missingPayload = (fork, sha, state) => { + const disabled = typeof state === 'string' && state.startsWith('disabled'); + const active = state === 'active'; + const advice = disabled + ? [ + '1. Enable workflows on your fork: open its **Actions** tab and confirm the prompt.', + '2. Push again. A push that changes only documentation or Markdown does not start', + ' the Rust workflow.', + ] + : active + ? [ + 'A push that changes only documentation or Markdown does not start the Rust', + 'workflow, and a run can take a moment to appear. This check keeps looking for one.', + ] + : [ + 'GitHub reports no Rust workflow on the fork, which usually means workflows are not', + 'enabled there. Enable them on the fork\'s **Actions** tab and push again. A push that', + 'changes only documentation or Markdown does not start the Rust workflow.', + ]; + return { + status: 'completed', + conclusion: disabled ? 'action_required' : active ? 'skipped' : 'neutral', + output: { + title: disabled ? 'Workflows are disabled on the fork' : 'No fork run for this commit', + summary: [ + `No Rust workflow run was found on ${fork.owner}/${fork.repo} for ${sha}.`, + '', + ...advice, + '', + HOW_UPDATED, + '', + `See ${GUIDE}`, + ].join('\n'), + text: JSON.stringify({ owner: fork.owner, repo: fork.repo, head_sha: sha, run_id: null }), + }, + }; + }; + + // The fork creates its run a few seconds after the push. + async function findRun(fork, sha, attempts) { + for (let attempt = 0; attempt < attempts; attempt++) { + try { + const { data } = await github.rest.actions.listWorkflowRuns({ + owner: fork.owner, + repo: fork.repo, + workflow_id: WORKFLOW, + head_sha: sha, + per_page: 10, + }); + const run = + data.workflow_runs.find((r) => r.event === 'push') || data.workflow_runs[0]; + if (run) return run; + } catch (error) { + // 404 when the fork has Actions disabled or lacks the workflow. + core.info(`No run list for ${fork.owner}/${fork.repo}: ${error.message}`); + } + if (attempt + 1 < attempts) { + await new Promise((resolve) => setTimeout(resolve, 5000)); + } + } + return null; + } + + // `active`, or `disabled_fork` until the owner enables workflows; + // null when the workflow cannot be read. + async function workflowState(fork) { + try { + const { data } = await github.rest.actions.getWorkflow({ + owner: fork.owner, + repo: fork.repo, + workflow_id: WORKFLOW, + }); + return data.state || null; + } catch (error) { + core.info(`No workflow state for ${fork.owner}/${fork.repo}: ${error.message}`); + return null; + } + } + + async function notify() { + const pr = context.payload.pull_request; + // Only fork PRs to main skip jobs upstream; release branches have + // no merge queue and keep the full run. + if (!isForkPrToMain(pr)) { + core.info('Not a fork pull request to main; nothing to mirror.'); + return; + } + const fork = { owner: pr.head.repo.owner.login, repo: pr.head.repo.name }; + const sha = pr.head.sha; + const run = await findRun(fork, sha, 12); + const payload = run + ? runPayload(fork, sha, run) + : missingPayload(fork, sha, await workflowState(fork)); + await github.rest.checks.create({ ...upstream, name: CHECK, head_sha: sha, ...payload }); + core.info(`Created ${CHECK} for ${sha}: ${payload.conclusion || payload.status}`); + } + + // Bring one pull request's check in line with its fork run. A check + // without a run keeps looking for one; a check with a run picks up + // progress and reruns. + async function syncCheck(pr) { + const { data } = await github.rest.checks.listForRef({ + ...upstream, + ref: pr.head.sha, + check_name: CHECK, + per_page: 5, + }); + const check = data.check_runs[0]; + if (!check || !check.output || !check.output.text) return; Review Comment: 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? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
