Copilot commented on code in PR #17316:
URL: https://github.com/apache/pinot/pull/17316#discussion_r2586896167
##########
.github/workflows/pinot_tests.yml:
##########
@@ -235,7 +265,13 @@ jobs:
verbose: true
- name: Generate Surefire Report
if: ${{ !cancelled() && github.ref == 'refs/heads/master' }}
- run: mvn surefire-report:report-only -P
github-actions,codecoverage,no-integration-tests || echo "Surefire report
generation failed, but continuing..."
+ continue-on-error: true
+ uses: nick-fields/retry@v3
+ with:
+ max_attempts: 3
+ retry_wait_seconds: 20
+ timeout_minutes: 120
+ command: mvn surefire-report:report-only -P
github-actions,codecoverage,no-integration-tests
Review Comment:
Using both `continue-on-error: true` and retry logic together may mask
persistent failures. The retry mechanism should handle transient errors, while
`continue-on-error: true` will suppress all failures including legitimate ones.
If the surefire report generation consistently fails after retries, this
configuration will hide that fact. Consider removing `continue-on-error: true`
to let the retry mechanism handle transient failures while surfacing persistent
issues.
##########
.github/workflows/pinot_tests.yml:
##########
@@ -57,11 +57,16 @@ jobs:
distribution: 'temurin'
cache: 'maven'
- name: Install pinot-dependency-verifier into repo
- run: |
- mvn clean install \
- -pl pinot-dependency-verifier \
- -am \
- -DskipTests
+ uses: nick-fields/retry@v3
+ with:
+ max_attempts: 3
+ retry_wait_seconds: 20
+ timeout_minutes: 120
Review Comment:
The retry configuration values (max_attempts: 3, retry_wait_seconds: 20,
timeout_minutes: 120) are duplicated across 13 different steps in this file and
across multiple workflow files. Consider extracting these as workflow-level
environment variables or constants to make future adjustments easier and
prevent inconsistencies.
##########
.github/workflows/pinot_tests.yml:
##########
@@ -366,7 +417,13 @@ jobs:
verbose: true
- name: Generate Surefire Report
if: ${{ !cancelled() && github.ref == 'refs/heads/master' }}
- run: mvn surefire-report:report-only -P github-actions,codecoverage ||
echo "Surefire report generation failed, but continuing..."
+ continue-on-error: true
+ uses: nick-fields/retry@v3
+ with:
+ max_attempts: 3
+ retry_wait_seconds: 20
+ timeout_minutes: 120
+ command: mvn surefire-report:report-only -P
github-actions,codecoverage
Review Comment:
Using both `continue-on-error: true` and retry logic together may mask
persistent failures. The retry mechanism should handle transient errors, while
`continue-on-error: true` will suppress all failures including legitimate ones.
If the surefire report generation consistently fails after retries, this
configuration will hide that fact. Consider removing `continue-on-error: true`
to let the retry mechanism handle transient failures while surfacing persistent
issues.
--
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]
