Self cleanup on stop hook #20

Merged
merged 2 commits into from Oct 19, 2016

Conversation

Projects
None yet
4 participants
Collaborator

chuckbutler commented Oct 19, 2016

This code path bloats the teardown time by about 20 seconds, but
successfully cleans up the interface and the associated files upon
executing the stop hook

Self cleanup on stop hook
This code path bloats the teardown time by about 20 seconds, but
successfully cleans up the interface and the associated files upon
executing the stop hook
Contributor

wwwtyro commented Oct 19, 2016

👍 This looks fine to me, not confident about merging until the cleanup discussion is resolved or @mbruzek or @marcoceppi give a thumbs up.

+ try:
+ subprocess.check_call(down)
+ subprocess.check_call(delete)
+ except subprocess.CalledProcessError:
@Cynerva

Cynerva Oct 19, 2016

Contributor

If we're going to ignore this exception here, can we at least log it for troubleshooting purposes?

@chuckbutler

chuckbutler Oct 19, 2016

Collaborator

👍 good shout. I'll add some debug messaging.

for f in files:
if os.path.exists(f):
+ hookenv.log('Removing {}'.format(f))
os.remove(f)
@mbruzek

mbruzek Oct 19, 2016

Contributor

Should we do this os.remove in a try/except to soldier on if we got a file we can not delete (for example if it was in use at the time of delete attempt)?

Contributor

mbruzek commented Oct 19, 2016

I am +1 to attempting to clean up after ourselves on the stop hook. My one comment was to handle exceptions when deleting files.

@mbruzek mbruzek merged commit 30c6d75 into juju-solutions:master Oct 19, 2016

Collaborator

chuckbutler commented Oct 19, 2016

It's a good comment. I filed #21 to track that comment as the code has landed.

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