Fix cuopt_mvn retry loop being silently skipped under set -e - #1823
Conversation
cuopt_mvn's retry loop ran `mvn ... | tee "${log}"` as a bare statement.
Callers (java/cuopt/scripts/test.sh) run under `set -euo pipefail`, so a
failing pipeline there aborted the function immediately, before the
retry logic that inspects PIPESTATUS ever ran. A Maven Central 429 hit
would fail on the very first attempt with none of the retry/backoff
behavior actually taking effect, as seen in job 98796785653 (PR #1810).
Guard the pipeline as the condition of an if so its failure is caught
by the retry loop instead of triggering the caller's set -e.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthroughThe Maven wrapper now guards the Maven and ChangesMaven retry handling
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized change restores the intended Maven retry behavior under strict shell error handling. No actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
CI Test Summary⏭️ All 5 test job(s) skipped. |
|
/merge |
Summary
cuopt_mvn's retry/backoff loop (java/cuopt/scripts/maven.sh, added in Java bindings for LP, MIP and QP #1524 to address java-build fails intermittently on Maven Central 429 rate limiting #1820) never actually ran:test.shinvokes it underset -euo pipefail, and the loop ranmvn ... | tee "${log}"as a bare statement rather than as the condition of anif/while. Withpipefailon, a failingmvnmakes the pipeline's exit status non-zero, andset -ethen kills the function immediately — before the code that readsPIPESTATUS[0]and decides whether to retry ever runs.mvnattempt hits a Maven Central 429, and the job fails immediately with none ofcuopt_mvn's retry log lines present, even though that branch already had the retry code.ifso its failure is caught by the loop instead of triggering the caller'sset -e. Verified locally that the loop now retries with backoff on a simulated 429 and still returns cleanly on success.Test plan
java-buildCI passesmvnreturning a 429-style error underset -euo pipefailnow producesmvn attempt 1/3 ...,attempt 2/3 ..., backoff sleeps, and a final failure only after exhausting retries (previously died on attempt 1 with no retry log at all)mvnreturning success still returns 0 immediately🤖 Generated with Claude Code