diff --git a/.github/actions/java-test/action.yaml b/.github/actions/java-test/action.yaml index a5ca166591c..662fa0d4971 100644 --- a/.github/actions/java-test/action.yaml +++ b/.github/actions/java-test/action.yaml @@ -96,7 +96,10 @@ runs: MAVEN_OPTS="-Xmx4G -Xms2G -DwildcardSuites=$MAVEN_SUITES -XX:+UnlockDiagnosticVMOptions -XX:+ShowMessageBoxOnError -XX:+HeapDumpOnOutOfMemoryError -XX:ErrorFile=./hs_err_pid%p.log" SPARK_HOME=`pwd` ./mvnw -B -Prelease install ${{ inputs.maven_opts }} - name: Upload crash logs if: failure() - uses: actions/upload-artifact@v6 + # These three stay on the plain action rather than + # ../upload-artifact-retry: a local action calling another local action + # is untested in this repo, and these only run on already-failing jobs. + uses: actions/upload-artifact@v7 with: name: crash-logs-${{ inputs.artifact_name }} path: "**/hs_err_pid*.log" @@ -110,13 +113,13 @@ runs: find . -name 'unit-tests.log' - name: Upload unit-tests.log if: failure() - uses: actions/upload-artifact@v6 + uses: actions/upload-artifact@v7 with: name: unit-tests-${{ inputs.artifact_name }} path: "**/target/unit-tests.log" - name: Upload test results if: ${{ inputs.upload-test-reports == 'true' }} - uses: actions/upload-artifact@v6 + uses: actions/upload-artifact@v7 with: name: java-test-reports-${{ inputs.artifact_name }} path: "**/target/surefire-reports/*.txt" diff --git a/.github/actions/upload-artifact-retry/action.yaml b/.github/actions/upload-artifact-retry/action.yaml new file mode 100644 index 00000000000..5bba453a907 --- /dev/null +++ b/.github/actions/upload-artifact-retry/action.yaml @@ -0,0 +1,145 @@ +# 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. + +name: "Upload Artifact (with retry)" +description: > + Drop-in replacement for actions/upload-artifact that retries the upload three + times. The artifact client only retries 429/500/502/503/504, so a + FinalizeArtifact answered "(403) Forbidden: Error from intermediary" fails the + step even though the content uploaded fine. There is no input to widen that + list, Actions has no built-in step retry, and third-party retry wrappers are + not on the Apache allowed-actions list. See .github/workflows/README.md. + + Retries force overwrite, which deletes the newest artifact carrying the name + before re-uploading. `name` must therefore identify exactly one producer in + the run: a reusable workflow ci.yml calls more than once has to qualify the + name with its version inputs. dev/ci/check-ci-config.py enforces that. + +# Inputs mirror actions/upload-artifact@v7 one for one. The boolean defaults +# have to be real 'true'/'false' strings because core.getBooleanInput() throws +# on an empty value; the numeric ones default to '' so "unset" round-trips. +inputs: + name: + description: 'Artifact name' + required: false + default: 'artifact' + path: + description: 'A file, directory or wildcard pattern that describes what to upload' + required: true + if-no-files-found: + description: "Behavior if no files are found: warn, error or ignore" + required: false + default: 'warn' + retention-days: + description: 'Days before the artifact expires (empty means repository default)' + required: false + default: '' + compression-level: + description: 'Zlib compression level 0-9 (empty means the action default)' + required: false + default: '' + overwrite: + description: 'Delete an existing artifact with the same name before uploading' + required: false + default: 'false' + include-hidden-files: + description: 'Include hidden files in the artifact' + required: false + default: 'false' + archive: + description: 'Zip the content before uploading' + required: false + default: 'true' + +# `||` yields the first non-empty operand, so this picks whichever attempt ran. +outputs: + artifact-id: + description: 'ID of the uploaded artifact' + value: ${{ steps.attempt-1.outputs.artifact-id || steps.attempt-2.outputs.artifact-id || steps.attempt-3.outputs.artifact-id }} + artifact-url: + description: 'Download URL of the uploaded artifact' + value: ${{ steps.attempt-1.outputs.artifact-url || steps.attempt-2.outputs.artifact-url || steps.attempt-3.outputs.artifact-url }} + artifact-digest: + description: 'SHA-256 digest of the uploaded artifact' + value: ${{ steps.attempt-1.outputs.artifact-digest || steps.attempt-2.outputs.artifact-digest || steps.attempt-3.outputs.artifact-digest }} + +runs: + using: "composite" + steps: + # continue-on-error keeps a failed attempt from failing the job while still + # recording outcome == 'failure', which the later attempts gate on. The last + # attempt omits it so a genuinely broken upload still fails loudly. + - name: Upload ${{ inputs.name }} (attempt 1 of 3) + id: attempt-1 + uses: actions/upload-artifact@v7 + continue-on-error: true + with: + name: ${{ inputs.name }} + path: ${{ inputs.path }} + if-no-files-found: ${{ inputs.if-no-files-found }} + retention-days: ${{ inputs.retention-days }} + compression-level: ${{ inputs.compression-level }} + overwrite: ${{ inputs.overwrite }} + include-hidden-files: ${{ inputs.include-hidden-files }} + archive: ${{ inputs.archive }} + + - name: Wait before retrying ${{ inputs.name }} + if: ${{ steps.attempt-1.outcome == 'failure' }} + shell: bash + run: | + echo "::warning::Upload of '${{ inputs.name }}' failed; retrying in 15s (attempt 2 of 3)." + sleep 15 + + # Retries force overwrite: attempt 1 may have created the server-side record + # before failing, and CreateArtifact rejects a duplicate name. The delete is + # best effort inside upload-artifact, so it no-ops when nothing exists, and + # the per-producer naming rule keeps the record it does find our own. + - name: Upload ${{ inputs.name }} (attempt 2 of 3) + id: attempt-2 + if: ${{ steps.attempt-1.outcome == 'failure' }} + uses: actions/upload-artifact@v7 + continue-on-error: true + with: + name: ${{ inputs.name }} + path: ${{ inputs.path }} + if-no-files-found: ${{ inputs.if-no-files-found }} + retention-days: ${{ inputs.retention-days }} + compression-level: ${{ inputs.compression-level }} + overwrite: 'true' + include-hidden-files: ${{ inputs.include-hidden-files }} + archive: ${{ inputs.archive }} + + - name: Wait before final retry of ${{ inputs.name }} + if: ${{ steps.attempt-1.outcome == 'failure' && steps.attempt-2.outcome == 'failure' }} + shell: bash + run: | + echo "::warning::Upload of '${{ inputs.name }}' failed again; retrying in 45s (attempt 3 of 3)." + sleep 45 + + - name: Upload ${{ inputs.name }} (attempt 3 of 3) + id: attempt-3 + if: ${{ steps.attempt-1.outcome == 'failure' && steps.attempt-2.outcome == 'failure' }} + uses: actions/upload-artifact@v7 + with: + name: ${{ inputs.name }} + path: ${{ inputs.path }} + if-no-files-found: ${{ inputs.if-no-files-found }} + retention-days: ${{ inputs.retention-days }} + compression-level: ${{ inputs.compression-level }} + overwrite: 'true' + include-hidden-files: ${{ inputs.include-hidden-files }} + archive: ${{ inputs.archive }} diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 8a8791e7118..4af0a704caa 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -103,6 +103,66 @@ dict at the top of `dev/ci/compute-changes.py`. The `changes` job in `needs.changes.outputs.`. When adding a new test suite or moving sources, update the relevant filter entry there. +A file that a job reads but that no filter lists is silent: the job skips, +and the edit merges with only `preflight` having looked at it. The shared +build inputs (`mvnw`, `.mvn/**`, the local composite actions) are pinned by +a routing table in `dev/ci/check-ci-config.py`, which `preflight` runs. + +## Artifact names must be unique per producer + +Artifact names are scoped to the workflow **run**, not to the calling +workflow. `ci.yml` calls `spark_sql_test_reusable.yml` once per Spark +version and `iceberg_spark_test_reusable.yml` once per Iceberg version, all +inside the same run, so an unqualified name like `native-lib-linux` would be +claimed by several producers at once. That breaks two things: + +- `download-artifact` resolves a name to the highest matching artifact ID. + Nothing ties it to the producer the consumer declared in `needs`. +- `upload-artifact` with `overwrite: true` deletes the newest record with + that name before uploading, which can be a sibling's finished artifact. + The retry wrapper below forces `overwrite` on attempts 2 and 3. + +So every artifact published by a reusable workflow that `ci.yml` calls more +than once carries its version inputs, e.g. +`native-lib-spark-4.1.3-jdk17`. `dev/ci/check-ci-config.py` enforces this, +and also that every `download-artifact` name is produced by an upload in the +same workflow. + +## Retrying flaky network operations + +**Maven.** `.mvn/maven.config` tunes the Maven Resolver HTTP transport: six +retries instead of three, `408/429/500/502/503/504` retryable instead of only +`429/503`, a 30s connect timeout and a 10 minute socket read timeout. The +wrapper pins `maven.multiModuleProjectDirectory` to the directory holding +`.mvn`, so one file covers every `mvnw` invocation in CI (including +`cd spark && ../mvnw ...`) with no per-workflow wiring. + +The Wagon transport (`-Dmaven.resolver.transport=wagon`, `maven.wagon.http.*`) +was evaluated and rejected: it is deprecated in Resolver 1.9 and removed in +Maven 4, its retry knobs mirror the native transport's, and its +service-unavailable retry strategy defaults to `none`, so adopting it would +first have to buy back the `429/503` retry we already get. The one thing it +can still do that the native transport cannot is shrink HttpClient's +non-retryable exception list (`retryHandler.class=default` plus +`retryHandler.nonRetryableClasses=...`), the only way to retry a connect or +read timeout. We size those timeouts not to fire instead. + +**Artifact upload.** `actions/upload-artifact` fails the job when +`FinalizeArtifact` returns `(403) Forbidden: Error from intermediary`, even +though the content already uploaded. Its client only retries +`429/500/502/503/504`, exposes no input to widen that, and Actions has no +built-in step retry. Use `./.github/actions/upload-artifact-retry` instead for +any artifact a later job consumes: same inputs and outputs, three attempts, +15s then 45s backoff. Attempts 2 and 3 force `overwrite: true`, so the name +must belong to exactly one producer in the run (see above). The diagnostic +uploads inside `./.github/actions/java-test` stay on the plain action, since a +local action calling another local action is untested here and those run only +on already-failing jobs. + +**Maven wrapper bootstrap.** `./.github/actions/java-test` retries +`./mvnw --version` with exponential backoff, so a failed download of the Maven +distribution does not surface as a test failure. + ## Branch protection Required-check names changed when these workflows were consolidated. The diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 59cc05215f1..889a5e33da9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -85,6 +85,9 @@ jobs: - name: Check missing suites run: python3 dev/ci/check-suites.py + - name: Check CI config invariants + run: python3 dev/ci/check-ci-config.py + - name: Install actionlint run: | curl -sSfL https://raw.githubusercontent.com/rhysd/actionlint/main/scripts/download-actionlint.bash | bash diff --git a/.github/workflows/docs.yaml b/.github/workflows/docs.yaml index 1ca45266304..6b245c45c07 100644 --- a/.github/workflows/docs.yaml +++ b/.github/workflows/docs.yaml @@ -26,7 +26,7 @@ jobs: build-docs: name: Build docs if: ${{ startsWith(github.repository, 'apache/') }} - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 steps: - name: Checkout docs sources uses: actions/checkout@v7 diff --git a/.github/workflows/iceberg_spark_test_reusable.yml b/.github/workflows/iceberg_spark_test_reusable.yml index 4b72cf47c2b..2d45d3ffaf7 100644 --- a/.github/workflows/iceberg_spark_test_reusable.yml +++ b/.github/workflows/iceberg_spark_test_reusable.yml @@ -103,9 +103,13 @@ jobs: key: ${{ runner.os }}-cargo-ci-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}-${{ hashFiles('native/**/*.rs') }} - name: Upload native library - uses: actions/upload-artifact@v7 + uses: ./.github/actions/upload-artifact-retry with: - name: native-lib-iceberg + # Version-qualified: ci.yml calls this workflow once per Iceberg + # version inside a single run, and artifact names are scoped to the + # run, not to the calling workflow. See "Artifact names must be + # unique per producer" in .github/workflows/README.md. + name: native-lib-iceberg-${{ inputs.iceberg-full }}-spark-${{ inputs.spark-full }}-jdk${{ inputs.java }} path: native/target/ci/libcomet.so retention-days: 1 @@ -127,7 +131,7 @@ jobs: - name: Download native library uses: actions/download-artifact@v8 with: - name: native-lib-iceberg + name: native-lib-iceberg-${{ inputs.iceberg-full }}-spark-${{ inputs.spark-full }}-jdk${{ inputs.java }} path: native/target/release/ - name: Build Comet run: | @@ -162,7 +166,7 @@ jobs: - name: Download native library uses: actions/download-artifact@v8 with: - name: native-lib-iceberg + name: native-lib-iceberg-${{ inputs.iceberg-full }}-spark-${{ inputs.spark-full }}-jdk${{ inputs.java }} path: native/target/release/ - name: Build Comet run: | @@ -197,7 +201,7 @@ jobs: - name: Download native library uses: actions/download-artifact@v8 with: - name: native-lib-iceberg + name: native-lib-iceberg-${{ inputs.iceberg-full }}-spark-${{ inputs.spark-full }}-jdk${{ inputs.java }} path: native/target/release/ - name: Build Comet run: | diff --git a/.github/workflows/pr_build_linux.yml b/.github/workflows/pr_build_linux.yml index d2f50bf09b6..718bd6ebd23 100644 --- a/.github/workflows/pr_build_linux.yml +++ b/.github/workflows/pr_build_linux.yml @@ -212,7 +212,7 @@ jobs: RUSTFLAGS: "-Ctarget-cpu=x86-64-v3 -Clink-arg=-fuse-ld=bfd" - name: Upload native library - uses: actions/upload-artifact@v7 + uses: ./.github/actions/upload-artifact-retry with: name: native-lib-linux path: native/target/ci/libcomet.so diff --git a/.github/workflows/pr_build_macos.yml b/.github/workflows/pr_build_macos.yml index 59479028e3c..17f47b71329 100644 --- a/.github/workflows/pr_build_macos.yml +++ b/.github/workflows/pr_build_macos.yml @@ -31,7 +31,7 @@ jobs: # Fast lint check - gates all other jobs (runs on Linux for cost efficiency) lint: name: Lint - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 container: image: amd64/rust steps: @@ -79,7 +79,7 @@ jobs: RUSTFLAGS: "-Ctarget-cpu=apple-m1" - name: Upload native library - uses: actions/upload-artifact@v7 + uses: ./.github/actions/upload-artifact-retry with: name: native-lib-macos path: native/target/ci/libcomet.dylib diff --git a/.github/workflows/pyarrow_udf_test.yml b/.github/workflows/pyarrow_udf_test.yml index 1a03962685d..82e61a51045 100644 --- a/.github/workflows/pyarrow_udf_test.yml +++ b/.github/workflows/pyarrow_udf_test.yml @@ -59,7 +59,7 @@ env: jobs: pyarrow-udf: name: PyArrow UDF (${{ matrix.name }}, JDK 17, Python 3.11) - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 strategy: fail-fast: false matrix: diff --git a/.github/workflows/spark_sql_test_reusable.yml b/.github/workflows/spark_sql_test_reusable.yml index 5d4f8f0473e..52b9d1208cf 100644 --- a/.github/workflows/spark_sql_test_reusable.yml +++ b/.github/workflows/spark_sql_test_reusable.yml @@ -54,13 +54,18 @@ jobs: # Build the native library AND pre-compile Spark sources + Test classes in a # single runner, then publish two artifacts the matrix consumes: - # - native-lib-linux: libcomet.so (~50 MB) + # - native-lib-spark--jdk: libcomet.so (~50 MB) # - jvm-compiled-spark--jdk: apache-spark.tar.gz (sources + # target/ + Zinc state, ~500 MB-1 GB) # Combining them avoids a second runner cold-start and an extra inter-job # artifact round-trip for the native lib, since the JVM build already # depends on it (the Comet Maven install bundles libcomet.so into the # Comet JAR before SBT resolves Spark's classpath). + # + # Both names carry the Spark/JDK version because ci.yml calls this workflow + # once per Spark version inside a single run, and artifact names are scoped + # to the run, not to the calling workflow. See "Artifact names must be unique + # per producer" in .github/workflows/README.md. build: name: Build Native + JVM Test Classes runs-on: ubuntu-24.04 @@ -104,9 +109,9 @@ jobs: key: ${{ runner.os }}-cargo-ci-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}-${{ hashFiles('native/**/*.rs') }} - name: Upload native library - uses: actions/upload-artifact@v7 + uses: ./.github/actions/upload-artifact-retry with: - name: native-lib-linux + name: native-lib-spark-${{ inputs.spark-full }}-jdk${{ inputs.java }} path: native/target/ci/libcomet.so retention-days: 1 @@ -156,11 +161,14 @@ jobs: apache-spark - name: Upload JVM compile artifact - uses: actions/upload-artifact@v7 + uses: ./.github/actions/upload-artifact-retry with: name: jvm-compiled-spark-${{ inputs.spark-full }}-jdk${{ inputs.java }} path: apache-spark.tar.gz retention-days: 1 + # Already gzipped: re-deflating ~1 GB at level 6 costs minutes for no + # size win, and a shorter upload is a smaller window for the flake. + compression-level: 0 spark-sql-test: needs: build @@ -190,7 +198,7 @@ jobs: - name: Download native library uses: actions/download-artifact@v8 with: - name: native-lib-linux + name: native-lib-spark-${{ inputs.spark-full }}-jdk${{ inputs.java }} path: native/target/release/ - name: Download JVM compile artifact uses: actions/download-artifact@v8 @@ -265,7 +273,7 @@ jobs: DEDICATED_JVM_SBT_TESTS: ${{ inputs.spark-short == '4.0' && 'org.apache.spark.sql.execution.datasources.parquet.ParquetFileFormatV1Suite,org.apache.spark.sql.execution.datasources.parquet.ParquetFileFormatV2Suite,org.apache.spark.sql.execution.datasources.orc.OrcSourceV1Suite,org.apache.spark.sql.execution.datasources.orc.OrcSourceV2Suite' || '' }} - name: Upload fallback log if: ${{ inputs.collect-fallback-logs }} - uses: actions/upload-artifact@v7 + uses: ./.github/actions/upload-artifact-retry with: name: fallback-log-spark-sql-${{ matrix.module.name }}-spark-${{ inputs.spark-full }}-jdk${{ inputs.java }} path: "**/fallback.log" @@ -284,6 +292,8 @@ jobs: run: | find ./fallback-logs/ -type f -name "fallback.log" -print0 | xargs -0 cat | sort -u > all_fallback.log - name: Upload merged fallback log + # Not wrapped in upload-artifact-retry: that local action needs a + # checkout, which this job deliberately skips. uses: actions/upload-artifact@v7 with: name: all-fallback-log-spark-${{ inputs.spark-full }}-jdk${{ inputs.java }} diff --git a/.mvn/maven.config b/.mvn/maven.config new file mode 100644 index 00000000000..6c17056dc72 --- /dev/null +++ b/.mvn/maven.config @@ -0,0 +1,38 @@ +# 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. +# +# Maven Resolver HTTP tuning, applied to every wrapper invocation: the wrapper +# pins `maven.multiModuleProjectDirectory` to the directory holding `.mvn`, so +# `./mvnw` from the root and `../mvnw` from `native/` or `spark/` all read this. +# Command-line flags still win. Rationale and the Wagon comparison live in +# .github/workflows/README.md. + +# Retries per request, for I/O errors and for the status codes below. Default 3. +-Daether.connector.http.retryHandler.count=6 + +# Retryable status codes. The default "429,503" fails the build on the first 502 +# or 504 from a CDN edge in front of Maven Central. Backoff is linear (5s, 10s, +# ...) and honours Retry-After. +-Daether.connector.http.retryHandler.serviceUnavailable=408,429,500,502,503,504 + +# TCP connect timeout, default 10s. HttpClient classifies a connect timeout as +# non-retryable, so the budget itself has to survive a busy runner. +-Daether.connector.connectTimeout=30000 + +# Idle gap allowed between bytes, not total transfer time. The 30 minute default +# lets a dead connection burn most of a job's budget. +-Daether.connector.requestTimeout=600000 diff --git a/dev/ci/check-ci-config.py b/dev/ci/check-ci-config.py new file mode 100644 index 00000000000..fb48400175b --- /dev/null +++ b/dev/ci/check-ci-config.py @@ -0,0 +1,157 @@ +# 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. + +# Guards two CI invariants that are silent when broken: +# +# 1. Change-filter routing. dev/ci/compute-changes.py decides which heavy +# jobs run. A file that a job depends on but that no filter lists makes +# that job skip, so the edit merges with only preflight having looked at +# it. The table below pins the routing for the shared build inputs. +# +# 2. Artifact-name uniqueness. Artifact names are scoped to the *run*, not +# to the calling workflow, and ci.yml calls the Spark SQL and Iceberg +# reusable workflows several times in one run. Two producers sharing a +# name make `download-artifact` pick by highest artifact ID rather than +# by `needs`, and make the forced `overwrite` on an upload retry delete +# a sibling's finished artifact. +# +# Run from the repository root: python3 dev/ci/check-ci-config.py + +import importlib.util +import re +import sys +from pathlib import Path + +WORKFLOWS = Path(".github/workflows") + +# Changed-file list -> the set of outputs compute-changes.py must report true. +# Every other output must be false. Keep one case per shared build input so a +# filter deletion cannot pass unnoticed. +BUILD_JOBS = { + "build_linux", + "build_macos", + "spark_3_4", + "spark_3_5", + "spark_4_0", + "spark_4_1", + "iceberg_1_8", + "iceberg_1_9", + "iceberg_1_10", + "iceberg_1_11", +} + +ROUTING_CASES = [ + # The Maven wrapper and its config feed every job that runs ./mvnw: the + # Linux/macOS builds, setup-spark-builder, and the Iceberg `mvnw install`. + ([".mvn/maven.config"], BUILD_JOBS), + ([".mvn/wrapper/maven-wrapper.properties"], BUILD_JOBS), + (["mvnw"], BUILD_JOBS), + # The upload wrapper is used by every producer of a shared artifact. + ([".github/actions/upload-artifact-retry/action.yaml"], BUILD_JOBS), + # Spot checks that the additions above did not widen unrelated routes. + (["docs/source/user-guide/overview.md"], {"docs"}), + (["native/core/benches/parquet_read.rs"], {"benchmark"}), +] + +# `uses:` values that publish an artifact, and the one that consumes it. +UPLOAD_USES = re.compile(r"uses:\s*(\./\.github/actions/upload-artifact-retry|actions/upload-artifact@)") +DOWNLOAD_USES = re.compile(r"uses:\s*actions/download-artifact@") +# The artifact name is the first `name:` key of the step's `with:` block. A +# following step starts with `- `, which distinguishes it from a `with:` key. +WITH_NAME = re.compile(r"^\s+name:\s*(\S.*?)\s*$") +NEW_STEP = re.compile(r"^\s*-\s") + + +def load_filters(): + spec = importlib.util.spec_from_file_location("compute_changes", "dev/ci/compute-changes.py") + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def check_change_filters(): + module = load_filters() + failures = [] + for files, expected_true in ROUTING_CASES: + for name, patterns in module.FILTERS.items(): + actual = module.matches(patterns, files) + expected = name in expected_true + if actual != expected: + failures.append( + f"{files}: expected {name}={str(expected).lower()}, " + f"got {str(actual).lower()} (see FILTERS in dev/ci/compute-changes.py)" + ) + for failure in failures: + print(f"change filter: {failure}") + return not failures + + +def artifact_names(path): + """Return ([upload names], [download names]) for one workflow file.""" + uploads, downloads = [], [] + lines = path.read_text(encoding="utf-8").splitlines() + for index, line in enumerate(lines): + if UPLOAD_USES.search(line): + bucket = uploads + elif DOWNLOAD_USES.search(line): + bucket = downloads + else: + continue + for following in lines[index + 1:]: + if NEW_STEP.match(following): + break # step ended without a `name:`; download-all, or the default + match = WITH_NAME.match(following) + if match: + bucket.append(match.group(1)) + break + return uploads, downloads + + +def check_artifact_names(): + ci = (WORKFLOWS / "ci.yml").read_text(encoding="utf-8") + call_counts = {} + for called in re.findall(r"uses:\s*\./\.github/workflows/(\S+)", ci): + call_counts[called] = call_counts.get(called, 0) + 1 + + failures = [] + for path in sorted(WORKFLOWS.glob("*.y*ml")): + uploads, downloads = artifact_names(path) + if call_counts.get(path.name, 0) > 1: + for name in uploads: + if "inputs." not in name: + failures.append( + f"{path}: artifact '{name}' is uploaded by a workflow ci.yml calls " + f"{call_counts[path.name]} times; qualify the name with an input " + f"(e.g. ${{{{ inputs.spark-full }}}}) so the parallel producers stay distinct" + ) + for name in downloads: + if name not in uploads: + failures.append( + f"{path}: artifact '{name}' is downloaded but never uploaded in the same " + f"workflow; a producer rename probably missed its consumer" + ) + for failure in failures: + print(f"artifact name: {failure}") + return not failures + + +if __name__ == "__main__": + ok = check_change_filters() + ok = check_artifact_names() and ok + if not ok: + sys.exit(1) + print("CI config checks passed") diff --git a/dev/ci/compute-changes.py b/dev/ci/compute-changes.py index 9b7cd2f1691..ca24c9dc241 100644 --- a/dev/ci/compute-changes.py +++ b/dev/ci/compute-changes.py @@ -43,6 +43,7 @@ ".github/actions/setup-builder/**", ".github/actions/java-test/**", ".github/actions/rust-test/**", + ".github/actions/upload-artifact-retry/**", "!**.md", "!native/core/benches/**", "!native/spark-expr/benches/**", @@ -65,6 +66,7 @@ ".github/workflows/pr_build_macos.yml", ".github/actions/setup-macos-builder/**", ".github/actions/java-test/**", + ".github/actions/upload-artifact-retry/**", "!**.md", "!native/core/benches/**", "!native/spark-expr/benches/**", @@ -110,6 +112,9 @@ ".github/workflows/spark_sql_test_reusable.yml", ".github/actions/setup-builder/**", ".github/actions/setup-spark-builder/**", + ".github/actions/upload-artifact-retry/**", + ".mvn/**", + "mvnw", ], "spark_3_5": [ "native/**/src/**", @@ -132,6 +137,9 @@ ".github/workflows/spark_sql_test_reusable.yml", ".github/actions/setup-builder/**", ".github/actions/setup-spark-builder/**", + ".github/actions/upload-artifact-retry/**", + ".mvn/**", + "mvnw", ], "spark_4_0": [ "native/**/src/**", @@ -154,6 +162,9 @@ ".github/workflows/spark_sql_test_reusable.yml", ".github/actions/setup-builder/**", ".github/actions/setup-spark-builder/**", + ".github/actions/upload-artifact-retry/**", + ".mvn/**", + "mvnw", ], "spark_4_1": [ "native/**/src/**", @@ -176,6 +187,9 @@ ".github/workflows/spark_sql_test_reusable.yml", ".github/actions/setup-builder/**", ".github/actions/setup-spark-builder/**", + ".github/actions/upload-artifact-retry/**", + ".mvn/**", + "mvnw", ], "iceberg_1_8": [ "native/**/src/**", @@ -193,6 +207,9 @@ ".github/workflows/iceberg_spark_test_reusable.yml", ".github/actions/setup-builder/**", ".github/actions/setup-iceberg-builder/**", + ".github/actions/upload-artifact-retry/**", + ".mvn/**", + "mvnw", ], "iceberg_1_9": [ "native/**/src/**", @@ -210,6 +227,9 @@ ".github/workflows/iceberg_spark_test_reusable.yml", ".github/actions/setup-builder/**", ".github/actions/setup-iceberg-builder/**", + ".github/actions/upload-artifact-retry/**", + ".mvn/**", + "mvnw", ], "iceberg_1_10": [ "native/**/src/**", @@ -227,6 +247,9 @@ ".github/workflows/iceberg_spark_test_reusable.yml", ".github/actions/setup-builder/**", ".github/actions/setup-iceberg-builder/**", + ".github/actions/upload-artifact-retry/**", + ".mvn/**", + "mvnw", ], "iceberg_1_11": [ "native/**/src/**", @@ -244,6 +267,9 @@ ".github/workflows/iceberg_spark_test_reusable.yml", ".github/actions/setup-builder/**", ".github/actions/setup-iceberg-builder/**", + ".github/actions/upload-artifact-retry/**", + ".mvn/**", + "mvnw", ], }