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

Mccalluc/delete tmp dirs on purge #79

Merged
merged 13 commits into from
Sep 20, 2017

Conversation

mccalluc
Copy link
Member

@mccalluc mccalluc commented Sep 18, 2017

Towards refinery-platform/refinery-platform#2160 (Will need to pin the version here and update the dependency there.)

Fix #68, Fix #67 (I think)

fix #61

@mccalluc
Copy link
Member Author

not the behavior I saw locally, but even apart from that, I'm doing things in the wrong order: should not delete directory while the container that depends on it is still running.

@mccalluc
Copy link
Member Author

@scottx611x : Tests pass locally. Started off just cleaning up the temp dir, and then continued into making the temp dir configurable. If this passes, I'll tag it, and work on a separate PR in refinery proper.

@mccalluc
Copy link
Member Author

Failure is related to tag mismatch, but I don't really understand it. I would merge and then release a new tag if necessary.

@@ -195,6 +193,7 @@ def test_purge(self):
))
self.assertEqual(1, self.count_containers())
self.assert_loads_eventually(url, 'Welcome to nginx!')
self.assertGreater(self.ls_tmp(), ls_tmp_orig)
Copy link
Member

Choose a reason for hiding this comment

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

ls_tmp_orig -> self.initial_tmp

Copy link
Member Author

Choose a reason for hiding this comment

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

filed #81

@scottx611x scottx611x merged commit 9e75a3a into master Sep 20, 2017
@scottx611x scottx611x deleted the mccalluc/delete-tmp-dirs-on-purge branch September 20, 2017 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants