sunchao commented on code in PR #6061:
URL: https://github.com/apache/datafusion-comet/pull/6061#discussion_r4057316063
##########
.github/workflows/pr_build_linux.yml:
##########
@@ -202,9 +202,36 @@ jobs:
restore-keys: |
${{ runner.os }}-java-maven-
+ # Split a Maven Central hiccup from a real lint violation. Resolving the
+ # scalafix plugin descriptor has failed mid-job before, leaving
+ # "No plugin found for prefix 'scalafix'" and a red lint that looks
+ # exactly like a violation (pull request #6035). The scalafix-syntactic
+ # job above solves the same problem by retrying the download and then
+ # linting offline; Maven has no equally safe offline mode here, because
+ # the goal resolves rule artifacts as it runs, so retry instead. Stop the
+ # moment scalafix reports actual errors, so a genuinely broken pull
+ # request is not rebuilt four times. Backoff matches
+ # .github/actions/maven-bootstrap: 10s, 20s, 40s plus 0-4s of jitter.
- name: Run scalafix check
run: |
- ./mvnw -B package -DskipTests scalafix:scalafix
-Dscalafix.mode=CHECK -Psemanticdb ${{ matrix.profile.maven_opts }}
+ set -o pipefail
+ for attempt in 1 2 3 4; do
+ if ./mvnw -B package -DskipTests scalafix:scalafix
-Dscalafix.mode=CHECK \
+ -Psemanticdb ${{ matrix.profile.maven_opts }} 2>&1 | tee
scalafix-check.log; then
+ exit 0
+ fi
+ if grep -q 'Scalafix invoked with errors' scalafix-check.log; then
Review Comment:
### Performance
[P2] Retry only recognized transient resolution failures
Could the retry branch require a known transient Maven resolution error, and
stop for other failures? This invocation includes `package`, so a deterministic
Scala compilation error exits nonzero without `Scalafix invoked with errors`.
Running the extracted step with that failure repeats the same Maven command
four times and adds 70–82 seconds of backoff before returning failure. This
repeats compilation for an unchanged broken input in every affected matrix row.
The existing bootstrap action avoids this by retrying only its download
operation. A focused case for a compiler failure alongside the resolution and
lint cases would guard the distinction.
##########
.github/workflows/pr_build_linux.yml:
##########
@@ -202,9 +202,36 @@ jobs:
restore-keys: |
${{ runner.os }}-java-maven-
+ # Split a Maven Central hiccup from a real lint violation. Resolving the
+ # scalafix plugin descriptor has failed mid-job before, leaving
+ # "No plugin found for prefix 'scalafix'" and a red lint that looks
+ # exactly like a violation (pull request #6035). The scalafix-syntactic
+ # job above solves the same problem by retrying the download and then
+ # linting offline; Maven has no equally safe offline mode here, because
+ # the goal resolves rule artifacts as it runs, so retry instead. Stop the
+ # moment scalafix reports actual errors, so a genuinely broken pull
+ # request is not rebuilt four times. Backoff matches
+ # .github/actions/maven-bootstrap: 10s, 20s, 40s plus 0-4s of jitter.
- name: Run scalafix check
run: |
- ./mvnw -B package -DskipTests scalafix:scalafix
-Dscalafix.mode=CHECK -Psemanticdb ${{ matrix.profile.maven_opts }}
+ set -o pipefail
+ for attempt in 1 2 3 4; do
+ if ./mvnw -B package -DskipTests scalafix:scalafix
-Dscalafix.mode=CHECK \
+ -Psemanticdb ${{ matrix.profile.maven_opts }} 2>&1 | tee
scalafix-check.log; then
Review Comment:
### Correctness
[P1] Keep the captured log outside the checkout
Could this log be written under `$RUNNER_TEMP` and read from that same path?
`tee scalafix-check.log` creates an untracked file on every successful run, and
the later `./dev/ci/check-working-tree-clean.sh` fails on any untracked file.
All four current Java lint rows already show `BUILD SUCCESS` followed by
`Working tree is not clean: ?? scalafix-check.log` ([Spark 4.0/JDK 21
log](https://github.com/apache/datafusion-comet/actions/runs/35516961938/job/106094710281)).
This makes Required Checks fail even when Scalafix passes.
--
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]
