-
Notifications
You must be signed in to change notification settings - Fork 389
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
Validate enum values #794
Validate enum values #794
Conversation
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
LGTM for Synthetics 👍
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.
Looks good, just a small but annoying comment.
@@ -5622,25 +5653,234 @@ func buildTerraformHostmapRequestStyle(datadogStyle datadogV1.HostMapWidgetDefin | |||
} | |||
|
|||
// Schema validation | |||
func validateWidgetPalette(val interface{}, key string) (warns []string, errs []error) { | |||
_, err := datadogV1.NewWidgetPaletteFromValue(val.(string)) | |||
if err != nil { |
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.
With a little bit of elbow grease, I feel that we could define a wrapper function that you can directly assign to ValidateFunc
. Seems non trivial. In the mean time, please inline the err check like if _, err := ..; err != nil
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.
That's awesome, one minor nit.
datadog/validators.go
Outdated
arg := reflect.ValueOf(val) | ||
outs := reflect.ValueOf(newEnumFunc).Call([]reflect.Value{arg}) | ||
err := outs[1].Interface() | ||
if err != nil { |
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.
Please inline the condition withe previous line.
Add a validate function to all enum values for clear error messages and fail early to avoid sending bad payloads to API
Fixes #787