Code review comment for lp:~viswesn/juju-ci-tools/ensure_provider_cleanup

>> Whatever you do, don't repeat yourself. Factor out the duplicate code.
<<Viswesn>> Currently we see most of the code is duplicated but then once we have list of resources to be cleaned up in coming days for each substrate then ensure_cleanup is going to be different for each substrate.

>> Don't catch Exception. Catch the specific exceptions that you expect to be raised.
<<Viswesn>> Now I need to address this by sending the unclean=[] to the callee function and then append the resource name + exception something like ["instance_id", "instance-001", "Interface failed"]

------------------------------
delete_detached_interfaces(self, security_group, unclean=None)
    if unclean is None:
          unclean=[]
    ...
    ....
     except Exception as ex:
          unclean.append(["instance_id", instance_name, type(ex).__name__, ex.args])
------------------------------

Chris, let me know your comments on this. We decided not to do so in our last call but then to catch the exact exception then I need to pass unclean to the callee method rather than having like try..catch.. statement on invoking terminate_instance.

>> You are providing a string to terminate_instances. I don't expect this to work;
<<Viswesn>> Yes, I agree and I will address this issue.

>> Are you certain that this script is not going to delete freshly-created resources?
<<Viswesn>> if ensure_cleanup has visibility to those freshly-created resource then yes but seems like it is not at present and it is up the substrate developer to bring this in to picture as part of ensure_cleanup.

I will start working on this code after getting comments from Chirs and Aaron.
Please let me know on this. Thanks.