Skip to content

ci: keep build/venv when the build retry nukes the build directory - #1814

Closed
sbryngelson wants to merge 2 commits into
MFlowCode:masterfrom
sbryngelson:fix/retry-keeps-venv
Closed

ci: keep build/venv when the build retry nukes the build directory#1814
sbryngelson wants to merge 2 commits into
MFlowCode:masterfrom
sbryngelson:fix/retry-keeps-venv

Conversation

@sbryngelson

Copy link
Copy Markdown
Member

Fixes #1813.

retry_build answered a failed first attempt with rm -rf build, which also removed build/venv. On Frontier the venv is fetched on the login node because compute nodes have no route to PyPI, so attempt 2 could never reinstall it, and classify-build-failure.sh read the resulting Failed to fetch https://pypi.org/... as a cluster-wide outage. One compile error on one branch then exited every Frontier job on every PR with code 78 for twenty minutes, and refreshed the marker each time it happened again. Today it was tripped twice (18:01 by a syscheck install timeout on another branch, 19:14 by a compile error on a probe branch) and skipped the Frontier lanes on #1805, #1807 and #1811.

The retry now removes everything under build/ except venv. Checked in a scratch tree: after two failed attempts only build/venv remains. Nothing else changes; a genuine PyPI failure on the login node still marks the breaker.

Copilot AI lite review requested due to automatic review settings September 3, 2026 21:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Pull request overview

Updates the CI build retry logic to preserve build/venv across retries, preventing Frontier compute-node jobs from failing due to inability to reach PyPI after the build directory is wiped.

Changes:

  • Add nuke_build() helper to delete everything under build/ except venv.
  • Replace rm -rf build with nuke_build in both retry paths.
  • Expand script header comment with Frontier/PyPI context tied to #1813.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/scripts/retry-build.sh Outdated
: "${MFC_BUILD_RETRY_DELAY:=30}"

nuke_build() {
find build -mindepth 1 -maxdepth 1 ! -name venv -exec rm -rf {} + 2>/dev/null || true

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in 608e137.

Comment thread .github/scripts/retry-build.sh Outdated
@@ -24,7 +30,7 @@ retry_build() {
echo "Post-build validation failed on attempt $attempt."
if [ $attempt -lt $max_attempts ]; then
echo " Nuking build directory before retry..."

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reworded in 608e137: the log now says the build directory is cleared with build/venv kept.

Comment thread .github/scripts/retry-build.sh Outdated
@@ -39,7 +45,7 @@ retry_build() {
fi
if [ $attempt -lt $max_attempts ]; then
echo " Build failed — nuking build directory before retry..."

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same wording change in 608e137.

@codecov

codecov Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.26%. Comparing base (8010057) to head (608e137).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1814   +/-   ##
=======================================
  Coverage   62.26%   62.26%           
=======================================
  Files          84       84           
  Lines       21558    21558           
  Branches     3188     3195    +7     
=======================================
  Hits        13423    13423           
  Misses       5937     5937           
  Partials     2198     2198           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sbryngelson

Copy link
Copy Markdown
Member Author

Folded into #1800 (2d1f453e, 608e137b cherry-picked), since the two changes interact and #1800 already touches the same failure.

One thing changed in the process: #1800 removes the cluster-wide outage breaker entirely, so the sentence here about a genuine PyPI failure still marking it no longer holds. The reason for keeping build/venv stands on its own though — a compute node has no route to PyPI, so removing it made every retry fail on a fetch that could not succeed. The rationale comment in retry-build.sh has been updated to say that instead.

Closing in favour of #1800; reopen if you'd rather land this separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Frontier CI: build retry deletes build/venv, reinstalls from a compute node, and records a false cluster-wide PyPI outage

2 participants