Skip to content

fix: retry rm operation in cleanup#12162

Merged
spalladino merged 1 commit intomasterfrom
ag/fix-enotempty
Feb 20, 2025
Merged

fix: retry rm operation in cleanup#12162
spalladino merged 1 commit intomasterfrom
ag/fix-enotempty

Conversation

@alexghr
Copy link
Contributor

@alexghr alexghr commented Feb 20, 2025

Fix #12141

@alexghr alexghr enabled auto-merge (squash) February 20, 2025 18:24
@ludamad ludamad disabled auto-merge February 20, 2025 18:25
if (!skipCleanup) {
await fs.rm(workingDirectory, { recursive: true, force: true });
try {
await fs.rm(workingDirectory, { recursive: true, force: true, maxRetries: 3, retryDelay: 100 });
Copy link
Collaborator

Choose a reason for hiding this comment

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

approving (and dequeuing) with request for log

Copy link
Contributor

Choose a reason for hiding this comment

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

You beat me to it

try {
await fs.rm(workingDirectory, { recursive: true, force: true, maxRetries: 3, retryDelay: 100 });
} catch {
// ignore cleanup errors
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not at least log them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't consider logging since this has only ever been seen in tests that fail.

@spalladino
Copy link
Contributor

Merging because I need this on my own PRs. Please add the logging statement later :-P

@spalladino spalladino merged commit 481d3be into master Feb 20, 2025
7 checks passed
@spalladino spalladino deleted the ag/fix-enotempty branch February 20, 2025 20:03
TomAFrench added a commit that referenced this pull request Feb 20, 2025
* master: (300 commits)
  fix(ci): don't have checks go green immediately (#12168)
  fix: ASSERTS that should throw (#12167)
  fix: retry rm operation in cleanup (#12162)
  chore: Fix linter errors (#12164)
  feat: Barretenberg C++ binary overhaul (#11459)
  fix: call install_hooks in bootstrap (#12159)
  chore: @aztec/stdlib pt. 3: aztec-address out of foundation (#12140)
  test: verify proving is resumed after broker crash (#11122)
  chore(ci3): update ci.md with swc notes (#12147)
  fix: don't try to get bench artifacts on external PR (#12157)
  feat: partial note handling in aztec-nr (#12122)
  fix: external fixes pt 2 (#12153)
  chore: fix message path (#12150)
  chore(ci3): refactor ci3.yml, fix external PR flow (#12037)
  fix: Do not try flushing txs in bot setup if not set (#12144)
  chore: Silence warns on invalid bootnode enr (#12135)
  fix: don't early-out on test fails (#12143)
  feat(avm): deduplicating event emitters (#12137)
  chore: @aztec/stdlib pt.2 -> remove @aztec/types (#12133)
  test: kill prover node and see it recover (#11118)
  ...
alexghr added a commit that referenced this pull request Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

investigate failure to delete dirs in orchestrator failure tests

3 participants