>> 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.
>> 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"]
------- ------- ------- ------- -- detached_ interfaces( self, security_group, unclean=None)
unclean= []
unclean. append( ["instance_ id", instance_name, type(ex).__name__, ex.args]) ------- ------- ------- --
delete_
if unclean is None:
...
....
except Exception as ex:
-------
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.