Code review comment for lp:~javier.collado/utah/provisioned_machine
Revision history for this message
| Max Brustkern (nuclearbob) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
bbfa235
(Get the code!)
Now for my thoughts on the code. I think since this class requires SSH, it would make sense to put it in the same file as SSHMixin. Right now that's ssh.py, but I'm not sure if we should have that in a separate file or if it should be moved back into provisioning.py. It was moved out for modularity, but we're still not using any machine that isn't SSH-based, so maybe it should just live in the main provisioning file. Either way, I'd be in favor of ProvisionedMachine living in the same file as SSHMixin.
Have you done any testing without your __del__ method in place? If the cleanup functions are written correctly, then if we haven't defined any cleanup actions for an instance of the class, nothing will happen when we call the cleanup functions, and we shouldn't need to skip them. If they are causing problems when we don't have any items to be cleaned up, I think that's probably something that should be fixed there.