-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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
Improve error message in case of invalid dynamic templates #60870
Improve error message in case of invalid dynamic templates #60870
Conversation
Include the attempted 'match_mapping_type' into the message, so that it is clearer that multiple validation attempts have occurred. Dynamic template validation was recently added via elastic#51233 and there was some confusion over the deprecation message itself. (in 7.x only deprecation warning will be omitted and from 8.0 an error will be returned)
Pinging @elastic/es-search (:Search/Mapping) |
Pinging @elastic/es-core-features (:Core/Features/Indices APIs) |
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. Two minor comments for your consideration.
String message = String.format(Locale.ROOT, "dynamic template [%s] has invalid content [%s]", | ||
dynamicTemplate.getName(), Strings.toString(dynamicTemplate)); | ||
String format = "dynamic template [%s] has invalid content [%s], " + | ||
"attempted to validate it with the following [match_mapping_type]: [%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.
Don't error messages usually use square brackets around values rather than names? In other words, should there be square brackets around match_mapping_type
?
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.
You're right, I don't think so.
Co-authored-by: Dan Hermann <danhermann@users.noreply.github.com>
@elasticmachine update branch |
@elasticmachine run elasticsearch-ci/docs |
Backporting elastic#60870 to 7.x branch. Include the attempted 'match_mapping_type' into the message, so that it is clearer that multiple validation attempts have occurred. Dynamic template validation was recently added via elastic#51233 and there was some confusion over the deprecation message itself. (in 7.x only deprecation warning will be omitted and from 8.0 an error will be returned)
Backporting #60870 to 7.x branch. Include the attempted 'match_mapping_type' into the message, so that it is clearer that multiple validation attempts have occurred. Dynamic template validation was recently added via #51233 and there was some confusion over the deprecation message itself. (in 7.x only deprecation warning will be omitted and from 8.0 an error will be returned)
Include the attempted 'match_mapping_type' into the message,
so that it is clearer that multiple validation attempts have occurred.
Dynamic template validation was recently added via #51233 and
there was some confusion over the deprecation message itself.
(in 7.x only deprecation warning will be omitted and from 8.0
an error will be returned)