andygrove opened a new pull request, #5852: URL: https://github.com/apache/datafusion-comet/pull/5852
## Which issue does this PR close? No issue — found while investigating a CI failure on #5850. ## Rationale for this change `Verify TPC-H Results` failed on #5850 at its `Build project` step, 105 seconds in and before anything was compiled. The check-run annotation names the cause: ``` https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.9.6/apache-maven-3.9.6-bin.zip ``` That's `./mvnw` downloading the Maven distribution itself — not a dependency, not a test. Nothing to do with TPC-H, and nothing to do with the PR it failed on (which only touched CI config). We already handle this. `./.github/actions/java-test` caches the distribution under `~/.m2/wrapper/dists` and retries `./mvnw --version` four times with exponential backoff. The problem is that five jobs in `pr_build_linux.yml` don't go through `java-test` — they call `./mvnw` directly — so they had neither the cache nor the retry: | Job | First Maven use | | --- | --- | | `lint-java` | scalafix check | | `build-spark-4-1` | compile, skip tests | | `celeborn-reflection-compatibility` | reflected-internals check | | `verify-benchmark-results-tpch` | the job that failed | | `verify-benchmark-results-tpcds` | same shape as TPC-H | The workflows README claims this failure mode is handled. It is, but only for jobs routed through `java-test`. This also matters for #5838: under a merge queue these jobs gate the queue, so a bootstrap blip would block *every* merge rather than costing one PR a re-run. ## What changes are included in this PR? Extracts the restore/retry/save sequence into `./.github/actions/maven-bootstrap` and calls it from all five jobs before their first Maven use. `java-test` keeps its inline copy rather than calling the new composite. A local action invoking another local action is deliberately avoided in this repo — the README already says as much about the artifact-upload wrapper — and converting `java-test` would be a bigger, riskier change than the bug warrants. The README wording is corrected to say which jobs are actually covered. ## How are these changes tested? CI on this PR is the test: all five jobs exercise the new step, and `lint-java`, `build-spark-4-1` and `celeborn-reflection-compatibility` are the fast ones that will show it working within a few minutes. Locally I verified with a script over the parsed workflow that every job invoking `./mvnw` now either uses `java-test` or has `maven-bootstrap` at a lower line number than its first Maven call — all five report OK, none regress. `actionlint`, `apache-rat:check`, `dev/ci/check-ci-config.py` and `prettier --check "**/*.md"` are clean. The retry path itself can't be exercised without an actual network failure; the logic is copied verbatim from `java-test`, where it has been in use already. ## Notes for reviewers Same gap exists in `ci.yml`'s RAT check and the direct `./mvnw` calls in `pr_benchmark_check.yml`, `pyarrow_udf_test.yml` and `iceberg_spark_test_reusable.yml`. I left those out to keep this reviewable against the failure that prompted it — happy to extend it if you'd rather do them all at once. `ci.yml`'s preflight is arguably the most valuable of those, since it gates the whole pipeline. -- 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]
