search

hashicorp / terraform-plugin-sdk

Terraform Plugin SDK enables building plugins (providers) to manage any service providers or custom in-house solutions
https://developer.hashicorp.com/terraform/plugin
Mozilla Public License 2.0
438 stars 232 forks source link

Show warnings when fixing names via StateFunc? #60

Open radeksimko opened 8 years ago

radeksimko commented 8 years ago

This is related to https://github.com/hashicorp/terraform/pull/4020 but also many other PRs which have been mimicking the AWS API behaviour in similar ways.

I understand the convenience for the user having existing resources and I understand that is the motivation for @stack72 and others to solve these issues that way. However I think that Terraform shouldn't be so silent about these fixes.

I think we should be more transparent and at least show warnings if the converted names differ from the original ones.

I'd personally like to use warnings in a way that it would allow us to turn on strict validation in the future (after a few versions, when we're sure enough users have been warned), but I understand the convenience for existing users has probably priority and that others may not like this (correct me if I'm wrong).

stack72 commented 8 years ago

@radeksimko I really like this idea - we should definitely be notifying the user that we are changing their variables so that they work with the APIs we are using

apparentlymart commented 8 years ago

I'm not sure I entirely agree with you. For an API that is case-insensitive, all of the different-cased versions of a value are semantically equivalent, and I don't really see the point in drawing attention to byte-wise diffs that don't represent a difference in meaning.

I would agree that we should never cross the line into making a change that results in a difference of meaning/behavior, but as long as we're just mimicking the same normalization that the underlying API was going to do I see this as a benefit (improved UX) rather than a drawback.

Have you encountered a real-world drawback to having these things silently normalized, from an end-user perspective?

(I know it has a very real developer drawback in that we have to keep adding all of these silly StateFuncs everywhere, but I think we've lowercased enough of these now that we could just have a reusable "make this lowercase" function that just wraps strings.ToLower to make its argument be interface{}, so we can write StateFunc: something.ToLowerCase instead of the same wrapper function each time.)

stack72 commented 8 years ago

@radeksimko / @apparentlymart was any discussion given to this outside of this issue? I feel this has sort of dropped off the face of the planet atm :)

paultyng commented 5 years ago

Easiest to accomplish this when we support diagnostics in protocol 5

radeksimko commented 5 years ago

This may also be related to https://github.com/hashicorp/terraform-plugin-sdk/issues/19

paultyng commented 4 years ago

74 is the tracking issue for diagnostics.