andygrove opened a new pull request, #5833: URL: https://github.com/apache/datafusion-comet/pull/5833
Backport of #5782 to `branch-1.0`. Cherry-picked from `80c8ec37af422db1952a1128cb23043c9bbfbb72`. The resulting diffstat is identical to the original commit's; the three conflicts were all context drift and are described under "What changes are included" below. Prompted by CI on #5820, where `Spark 4.2, JDK 17 [shuffle]` failed before running a single test: Maven could not resolve `git-commit-id-maven-plugin`'s dependencies because `https://repo.eclipse.org/...` returned `Network is unreachable`, and the default resolver gives up after 3 attempts. That is exactly the class of flake this change is meant to absorb, and `branch-1.0` does not have it. ## Which issue does this PR close? Part of #5489 on `branch-1.0`. Listed as a backport candidate in #5815. ## Rationale for this change `branch-1.0` will keep receiving 1.0.x backports for a while, and every one of them pays for the two unretried network flakes that #5782 fixed on `main`: - **Maven.** Maven 3.9.6's resolver retries 3 times and treats only `429/503` as retryable, so a `502`/`504` from the CDN in front of Maven Central, or a plain unreachable host, fails the build on first hit. `branch-1.0` pins the same Maven 3.9.6 as `main`, so it has the same defaults. - **Artifact upload.** `@actions/artifact` classifies a `403` from `FinalizeArtifact` as non-retryable and fails the job even though the content uploaded fine. Re-running a flaked job is a poor substitute here: the Spark SQL and Iceberg jobs are 30-70 minutes each, and on a release branch a spurious red is easy to mistake for a real backport regression. ## What changes are included in this PR? The change is the original one — see #5782 for the full rationale, including why the Wagon transport was evaluated and rejected. In short: `.mvn/maven.config` (new) raises resolver retries 3 -> 6 and widens the retryable statuses to `408,429,500,502,503,504`; `.github/actions/upload-artifact-retry` (new) wraps `actions/upload-artifact` with three attempts; artifact names become per-producer; `dev/ci/compute-changes.py` learns the new paths; and `dev/ci/check-ci-config.py` (new, run from `preflight`) pins both invariants. Three conflicts, all resolved by keeping `branch-1.0`'s surroundings and taking only what #5782 itself changed: - `.github/actions/java-test/action.yaml`: kept `branch-1.0`'s `if: ${{ inputs.upload-test-reports == 'true' }}` on the test-reports upload. The `!cancelled() &&` guard that appears on `main` comes from a later commit and is only context in #5782's diff, which bumps that step `v6 -> v7` and nothing else. - `.github/workflows/ci.yml`: added only the `check-ci-config.py` preflight step. The three steps adjacent to it on `main` run `check-benchmark-runner.py`, `pr-type-label.test.mjs` and `test-iceberg-shards.py`, none of which exist on `branch-1.0`. - `dev/ci/compute-changes.py`: added only this commit's three filter entries (`.github/actions/upload-artifact-retry/**`, `.mvn/**`, `mvnw`). The `dev/ci/iceberg-test-shards.gradle`, `check-iceberg-shards.py` and `test-iceberg-shards.py` entries in the same four hunks refer to files that do not exist on `branch-1.0`. Nothing was dropped as inapplicable. The preconditions the change is built around all hold here: `ci.yml` calls `spark_sql_test_reusable.yml` and `iceberg_spark_test_reusable.yml` four times each, which is the shared-artifact-name hazard the per-producer naming addresses; `compute-changes.py` has exactly the twelve `FILTERS` keys `check-ci-config.py`'s routing table expects; and all six sites that now use the local `upload-artifact-retry` action are in jobs that check out the repository, while `merge-fallback-logs` (which deliberately does not) correctly stays on the plain action. ## How are these changes tested? Verified locally on `branch-1.0` with Maven 3.9.6 / JDK 17: - All four settings reach Maven: `help:evaluate` returns `6`, `408,429,500,502,503,504`, `30000` and `600000`. Also checked from `spark/` via `../mvnw`, so the wrapper's `maven.multiModuleProjectDirectory` pinning works here as it does on `main`. - `dev/ci/check-ci-config.py` passes, and is not passing vacuously: its `BUILD_JOBS` set matches `branch-1.0`'s `FILTERS` keys exactly, so every routing case is actually evaluated. - `dev/ci/check-suites.py`, `actionlint -color --shellcheck=off` and `apache-rat:check` (0 unknown licenses) all pass. - `git diff --stat` against the original commit is byte-identical, and a diff of the two patches shows no content delta beyond the three adaptations above. The retry paths only execute on a real flake, so CI here should show attempt 1 succeeding. ## Are there any user-facing changes? No. CI configuration only; nothing ships in a release artifact. -- 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]
