-
Notifications
You must be signed in to change notification settings - Fork 68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: delete ISO even if already attached #131
feat: delete ISO even if already attached #131
Conversation
@johnrichardrinehart Thanks for submitting this - I'll try to to get this by end of the week if not I'll have a review for ya by monday |
@ddymko Absolutely no rush. Thanks for the ping. |
refactor: use single loop for instance association with ISO Id
refactor: made error messages more consistent
…the break condition clearer semantically
fix: return from resourceVultrIsoDelete as soon as ISO is detached and deleted successfully
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally looks good
@johnrichardrinehart Thanks for taking the time to implement this. We have some other features we want to release along side this. We are going to tag this in a 2.3.0 milestone and plan to release next week |
@ddymko Thanks for taking the time to work with me on a review. I'm much happier since we made the changes you suggested. I still don't like my solution for detecting if it's still attached (https://github.com/vultr/terraform-provider-vultr/pull/131/files#diff-9440d6827d830f384c9118b4fc75b2aea56b1c826f826fc780dbef8c208831f5R122). But, without error constants/values it's not going to look great. I'm excited for the release and I'll be using this feature myself. Hopefully it benefits someone else :) . |
Description
It's currently impossible to change the URL of an ISO resource that's attached to an instance. This PR allows for the deletion of an ISO that's attached to an instance at deletion-time.
Code/Logical Structure
resourceVultrIsoDelete
to attempt to delete again.Related Issues
#128
Checklist:
terraform
infrastructure.