-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Return appropriate error on null dims update instead of npe #125716
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
Return appropriate error on null dims update instead of npe #125716
Conversation
|
Pinging @elastic/es-search-relevance (Team:Search Relevance) |
|
Hi @benwtrent, I've created a changelog YAML for you. |
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
|
|
||
| return XContentMapValues.nodeIntegerValue(o); | ||
| }, m -> toType(m).fieldType().dims, XContentBuilder::field, Object::toString).setSerializerCheck((id, ic, v) -> v != null) | ||
| }, m -> toType(m).fieldType().dims, XContentBuilder::field, Objects::toString).setSerializerCheck((id, ic, v) -> v != null) |
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.
nit: this is kinda gross to read; might help in the future to see that default value on a separate line which might have maybe might have made it more obvious that Objects::toString is needed here more similar to how this is formatted in DateScriptFieldType:65 which breaks this Parameter instantiation out into an arg per line.
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
💔 Backport failed
You can use sqren/backport to manually backport by running |
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
…125716) Calling `Object::toString` was trying to call `null.toString()`, really it should have been `Objects::toString`, which accepts `null`. closes: elastic#125713 (cherry picked from commit dd58b0b)
…125716) Calling `Object::toString` was trying to call `null.toString()`, really it should have been `Objects::toString`, which accepts `null`. closes: elastic#125713 (cherry picked from commit dd58b0b)
…125716) Calling `Object::toString` was trying to call `null.toString()`, really it should have been `Objects::toString`, which accepts `null`. closes: elastic#125713
Calling
Object::toStringwas trying to callnull.toString(), really it should have beenObjects::toString, which acceptsnull.closes: #125713