Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: make post unmount even if buildkitd is no longer present #69

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

adityamaru
Copy link

@adityamaru adityamaru commented Dec 11, 2024

Also increase retries when trying to unmount the buildkit directory. Retry up to 3 seconds now, previously we were only retrying 3 times with a 100ms backoff.


Important

Increase unmount retries and handle missing buildkitd processes in src/main.ts.

  • Behavior:
    • Increase unmount retries from 3 to 10 in actionsToolkit.run and shutdownBuildkitd().
    • Extend retry backoff from 100ms to 300ms.
    • Handle cases where buildkitd is not present by checking process existence before attempting shutdown.
  • Error Handling:
    • Add debug logs for no lingering buildkitd processes.
    • Improve error messages during cleanup in actionsToolkit.run.

This description was created by Ellipsis for de0451e. It will automatically update as commits are pushed.

Also increase retries when trying to unmount the buildkit directory.
Retry up to 3 seconds now, previously we were only retrying 3 times
with a 100ms backoff.
Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to de0451e in 31 seconds

More details
  • Looked at 99 lines of code in 1 files
  • Skipped 2 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. src/main.ts:384
  • Draft comment:
    The retry logic for unmounting the buildkit directory is inconsistent. In the main function, it retries 10 times with a 300ms delay, but here it retries only 3 times with a 100ms delay. Consider updating this to match the main function's logic for consistency.
          for (let attempt = 1; attempt <= 10; attempt++) {
  • Reason this comment was not posted:
    Comment was on unchanged code.

Workflow ID: wflow_laKtjXvA0vhteN7W


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@adityamaru adityamaru merged commit 5b9a178 into master Dec 11, 2024
3 checks passed
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.

1 participant