-
Notifications
You must be signed in to change notification settings - Fork 6
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
Clear build dir on thumbot retry #61
Conversation
Thanks @davidx! Settings---
minimum_reviewers: 1
build_steps:
- bundle install
- bundle exec ruby test/test.rb
merge: true
org_mode: true
timeout: 1805
shell: "/bin/bash"
env:
BUNDLE_GEMFILE: Gemfile |
Looks good! 👍✅ MERGE
✅ BUNDLE_INSTALL
✅ BUNDLE_EXEC_RUBY_TEST/TEST.RB
⬜ 0 of 1 Code reviews from organization basho-labs |
Looks good! 👍✅ MERGE
✅ BUNDLE_INSTALL
✅ BUNDLE_EXEC_RUBY_TEST/TEST.RB
⬜ 0 of 1 Code reviews from organization basho-labs |
@@ -920,6 +920,10 @@ def comment_code_approvals | |||
code_reviews | |||
end | |||
|
|||
def remove_build_dir | |||
FileUtils.mv(@build_dir, "#{@build_dir}.#{DateTime.now.strftime("%s")}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps move to /tmp instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh, its already in tmp :) (by default, this way it'll move it relative to the toplevel build dir. /tmp/thumbs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@paegun ^^ .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see build_dir as /tmp/thumbs/<unique_dir_name> , so this will technically move the directory to be a child of the build directory, so would cause problems for projects that recursively perform actions w/i their build directory. I think it is better to move the directory up to /tmp/thumbs/garbage or the like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, this will move the directory to a new name,
/tmp/thumbs/build_guid gets moved to directory /tmp/thumbs/build_guid.timestamp
So completely separate directories.
But for cleanliness and future dealing with the output cruft, I agree it should be in a special place like /tmp/thumbs/archive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As long as it's not a child, :)
+1 |
✅ 1 of 1 Code reviews from organization basho-labs
|
Merging and closing this pr |
Successfully merged basho-labs/thumbs/pulls/61 (b73c84f on to master) ---
:sha: 6e2b74211215a82a2ccbee68dfef8bb65756caf4
:merged: true
:message: Pull Request successfully merged
|
This blows away the build directory during a retry so that a completely fresh state can be attained.
For potential forensics, the build dir will actually be moved to a backup directory, to be truncated from disk at a later time.
This pr also adds behavior that instead of updating the existing build status, it now creates a new build status comment and removes the old one.
example: davidx/prtester#330