> blamar: the rest of delete and force_delete are identical, but the
> preconditions are different. Both require different states to perform the
> action requested. This is to ensure that if you delete an instance twice, it
> doesn't get hard deleted. And that if the forceDelete action is called, it
> doesn't do a hard delete unless it's already in a soft delete state.
>
> The logic can be pushed up into the OS API level, but that leaves the compute
> API level open to being called incorrectly.
>
> An alternative would be to pass a flag to delete() to differentiate between
> the two. Or the common logic could be split out to a _delete() method leaving
> delete() and force_delete() to just check with the correct state.
>
> After thinking about it, I'm leaning towards a flag to delete().
I'm not a huge fan of binary flags so I'd lean towards the _delete() method, but either way the code cleanup would be welcome so it's your call.
I do agree with Waldon's suggestion of FLAGS.enable_instance_soft_delete & FLAGS.reclaim_instance_interval just because it is a little awkward to have reclaim_instance_interval = 0 disable soft deletes in my opinion.
> blamar: the rest of delete and force_delete are identical, but the
> preconditions are different. Both require different states to perform the
> action requested. This is to ensure that if you delete an instance twice, it
> doesn't get hard deleted. And that if the forceDelete action is called, it
> doesn't do a hard delete unless it's already in a soft delete state.
>
> The logic can be pushed up into the OS API level, but that leaves the compute
> API level open to being called incorrectly.
>
> An alternative would be to pass a flag to delete() to differentiate between
> the two. Or the common logic could be split out to a _delete() method leaving
> delete() and force_delete() to just check with the correct state.
>
> After thinking about it, I'm leaning towards a flag to delete().
I'm not a huge fan of binary flags so I'd lean towards the _delete() method, but either way the code cleanup would be welcome so it's your call.
I do agree with Waldon's suggestion of FLAGS.enable_ instance_ soft_delete & FLAGS.reclaim_ instance_ interval just because it is a little awkward to have reclaim_ instance_ interval = 0 disable soft deletes in my opinion.