-
Notifications
You must be signed in to change notification settings - Fork 233
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
helper/schema: Adjusted validation error messages #755
Conversation
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.
I'm noticing some inconsistencies here about when "attribute" is used and when "argument" is used. Is there an underlying system at work here that I'm not picking up on?
helper/schema/schema.go
Outdated
@@ -1453,7 +1453,7 @@ func (m schemaMap) validate( | |||
if err != nil { | |||
return append(diags, diag.Diagnostic{ | |||
Severity: diag.Error, | |||
Summary: "ExactlyOne", | |||
Summary: "Missing required argument", |
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.
This may not be strictly correct, as I believe this would error if none of the arguments were selected, or if more than one of the arguments were selected.
Summary: "Missing required argument", | |
Summary: "Invalid number of arguments", |
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.
Ahh yes, true: this can actually be describing two different problems:
- You didn't set any of the arguments in the set.
- You set more than one of the arguments in the set.
I think I'd prefer to change this to "Invalid combination of arguments", because "number" here makes it feel to me like there's the wrong number of arguments in this block in total, rather than the wrong number of arguments within this particular set of arguments that isn't directly visible to the user except in this error message. I will change it, though!
Regarding argument vs. attribute: "attribute" is the general term for something you could access on the resulting object using attribute syntax, like We adopted this terminology in response to the large amount of existing precedent in provider documentation, like in This also speaks a bit to my comment about referring to the "unconfigurable attributes" in my initial writeup: we don't have any established terminology in the user model for what's left if you subtract "arguments" from "attributes", and so I coined "unconfigurable attributes" here as an informal way to refer to those. |
Some of the validation error messages were written with the provider developer audience rather than the end-user audience in mind, and so in helping various folks in the community forum, etc I've occasionally seen folks be confused as to what these messages mean and what to do about them. Since I was here anyway I also did some general review of the terminology used and adjusted some things that were not necessarily confusing but that were using different terminology than Terraform Core would use for a similar problem. However, for many of them I didn't change them heavily because in many cases these validation errors are masked by equivalent schema-based validation in Terraform Core itself anyway, and so these can often be seen only by Terraform v0.10/v0.11 users.
a22a3a3
to
576f351
Compare
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.
Thanks @apparentlymart, I think this is much clearer for users and I'm happy to see the consistency in terms get propagated :)
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Some of the validation error messages were written with the provider developer audience rather than the end-user audience in mind, and so in helping various folks in the community forum, etc I've occasionally seen folks be confused as to what these messages mean and what to do about them.
The main cases I came here to address were:
schema.Schema
type, which is not something we expect end-sers to be familiar with.Since I was here anyway I also did some general review of the terminology used and adjusted some things that were not necessarily confusing but that were using different terminology than Terraform Core would use for a similar problem. However, for many of them I didn't change them heavily because in many cases these validation errors are masked by equivalent schema-based validation in Terraform Core itself anyway, and so these can often be seen only by Terraform v0.10/v0.11 users.