helper/schema: Exists API

[mitchellh]

This adds Exists to the helper/schema.Resource. This is called prior to Read and handles the situation that a resource no longer exists.

I added this as a formal CRUD-like operation because we get so many bugs where Read isn't handling the case where a resource doesn't exist properly. By documenting this case and exposing it as a first-class callback, I hope that resources in the future will be of high quality.

For backwards compatibility, this is an optional field, but we don't document it as such in the website since we want to encourage its use.

/cc @pearkes

[knuckolls]

+1

[bitglue]

What of the case where implementing Exists requires an additional API call when just Read would have been sufficient? I agree with most of the reasoning here, but won't encouraging providers to implement Read and Exists separately double the time it takes to do a refresh?

[frntn]

For backwards compatibility, this is an optional field, but we don't document it as such in the website since we want to encourage its use

@mitchellh with this choice the documentation will no more reflects the code. This must not be.
Users shouldn't have to read the code instead of the documentation (even if they shoud we cannot expect it to be the default user behaviour)

I would suggest the following :

• doc: document it as optional + explicitely encourage its usage in the doc (so people will actually know it's important)
• run: warn at when it's missing + doc link ( + say it will be mandatory start version 0.4 or so... ?)

(that's a conceptual comment as I didn't even deep look at that feature. Hope this still make sense)

[mitchellh]

@bitglue Good point, in this case there will be two round trips. I'm not sure if there is a better way around this, since I think having 1 round trip but forgetting to handle this case is more annoying.

@frntn Sorry I don't understand. It is only optional for backwards compatibility reasons. The "code" will show its optional, but its not truly meant to be. The documentation reflects the real state: it is required. It happens to work without specifying it, but the documentation says that it is required, so you do this at your own peril.

[frntn]

My concern was about the contradiction between "The 'code' will show its optional" vs "The documentation reflects the real state: it is required".

I am still not ok with that, but thanks to your last sentence I understand now what you mean. It makes sense.

Thanks.

[bitglue]

@mitchellh I agree: I've already encountered this bug on a couple resources, and made the same mistake in my first provider implementation. However, the current mechanism of SetId("") in the read function works fine as far as I know, and in most cases does not require the provider to make two API calls. If there's no technical reason that can't continue to work, I think it merits mentioning in the documentation that if Exists is not implemented, SetId("") works too. I agree I'd rather have a correct provider than a broken one, but I'd also like fast providers to be possible without relying on undocumented behavior.

Another approach would be making object existence explicit in some other way which isn't a separate function. For example, Read could additionally return a boolean, or there could be some method on ResourceData which if not called in Read results in an error. In other words, make a no-op Read an error, and require it to state explicitly if the object still exists or not. This could be extended to attributes as well: I wouldn't be surprised if there are at least a few bugs lurking where a change to some attribute doesn't get noticed by Terraform on refresh.

[mitchellh]

@frntn I think the solution would be to put in the code (via a comment above the field) that it is required. So even if the logic shows its optional, the docs both on the site and in the code show its not.

@bitglue Your latter two ideas are not bad. I'll sit on that one. I don't think its too late to undo Exists, so let me think about it.

[ghost]

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.