Skip to content
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

resources update methods #95

Closed
treywelsh opened this issue Dec 30, 2020 · 1 comment · Fixed by #96
Closed

resources update methods #95

treywelsh opened this issue Dec 30, 2020 · 1 comment · Fixed by #96

Comments

@treywelsh
Copy link
Collaborator

treywelsh commented Dec 30, 2020

Update methods doesn't work all the same way there is some refactoring to do.

Some of them use Partial and SetPartial methods, some others not.
As a side note, SetPartial is deprecated in later version of terraform SDK (but we don't use the SDK for now): https://www.terraform.io/docs/extend/guides/v2-upgrade-guide.html#removal-of-helper-schema-resourcedata-setpartial

Some of them don't call the read method at the end of the update.

I modified the VM update method in a previous PR (NIC update) but I still have some points to figure out around partial update of the state in case of update failure

treywelsh added a commit to treywelsh/terraform-provider-opennebula that referenced this issue Dec 31, 2020
treywelsh added a commit to treywelsh/terraform-provider-opennebula that referenced this issue Dec 31, 2020
@treywelsh
Copy link
Collaborator Author

treywelsh commented Feb 3, 2021

Some providers seems to remove Partial and SetPartial calls, some other keeps Partial calls only.

Calling Partial may have an impact in case of an error during an update operation, see doc linked above and this issue for more details.

From my understanding, a diff may be applied to the state, even in case of an error during update operation, for values that aren't Set in Read resource operation.
There are some attributes in this provider that could be impacted but for now I don't see any case were this would be legitimate to keep a Partial call.

For instance VM context should be impacted (or some other attributes only used at VM creation time), but to me, it seems to be an other problem, we should just read it or forbid the user to update it with a check.
This issue is more appropriated for this discussion: it point diff applied in state with no effect on the cloud provider side.

The PR I proposed for now remove all Partial and SetPartial calls.

treywelsh added a commit to treywelsh/terraform-provider-opennebula that referenced this issue Feb 5, 2021
treywelsh added a commit to treywelsh/terraform-provider-opennebula that referenced this issue Feb 5, 2021
@treywelsh treywelsh linked a pull request Feb 23, 2021 that will close this issue
treywelsh added a commit to treywelsh/terraform-provider-opennebula that referenced this issue Feb 23, 2021
treywelsh added a commit to treywelsh/terraform-provider-opennebula that referenced this issue Feb 23, 2021
@jaypif jaypif added this to the 0.4.0 milestone Jun 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants