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

Deprecate (helper/schema.ResourceData).Partial and (helper/schema.ResourceData).SetPartial #312

Closed
bflad opened this issue Feb 4, 2020 · 4 comments · Fixed by #317
Closed
Assignees
Labels
bug Something isn't working documentation Improvements or additions to documentation
Milestone

Comments

@bflad
Copy link
Contributor

bflad commented Feb 4, 2020

SDK version

v1.6.0

Relevant provider source code

func exampleThingUpdate(d *schema.ResourceData, meta interface{}) error {
	// ...

	d.Partial(true)

	if d.HasChange("widget") {
		// ... update widget
		d.SetPartial("widget")
	}

	d.Partial(false)

	// ...
}

Expected Behavior

That coding d.Partial() and d.SetPartial() into the update logic is beneficial.

Actual Behavior

Their usage is only beneficial in very small edge cases, e.g. when those attributes are not refreshed on the next run after an error occurs (Terraform is called with -refresh=false). As another side effect, only the configuration value is passed through into the state, not the potentially different API result after re-read, which could be incorrect.

Most resources should already be coded to perform a full refresh of all possible attribute values for drift detection, so partial state updates would (and should 😄 ) be overwritten on the next Terraform run.

Steps to Reproduce

  1. terraform apply (with error)
  2. terraform apply -refresh=false (not recommended)

References

@bflad bflad added the bug Something isn't working label Feb 4, 2020
@paultyng paultyng added the documentation Improvements or additions to documentation label Feb 4, 2020
@paultyng paultyng added this to the v2.0.0 milestone Feb 4, 2020
@paultyng
Copy link
Contributor

paultyng commented Feb 4, 2020

I added the 2.0.0 milestone to this as I think we can deprecate in 1.x and then remove in the 2.x line.

@ewbankkit
Copy link
Contributor

The Writing Custom Providers chapter of Extending Terraform -
https://www.terraform.io/docs/extend/writing-custom-providers.html#error-handling-amp-partial-state - should be updated to remove references to partial mode.

@paultyng
Copy link
Contributor

Thanks @ewbankkit, I opened an issue over on the website to do so.

@ghost
Copy link

ghost commented Mar 8, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Mar 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants