-
Notifications
You must be signed in to change notification settings - Fork 25.2k
[ML] Adding placeholder functionality for custom model request logic #127271
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
[ML] Adding placeholder functionality for custom model request logic #127271
Conversation
@@ -57,9 +57,11 @@ dependencies { | |||
implementation 'io.grpc:grpc-context:1.49.2' | |||
implementation 'io.opencensus:opencensus-api:0.31.1' | |||
implementation 'io.opencensus:opencensus-contrib-http-util:0.31.1' | |||
implementation "org.apache.commons:commons-lang3:${versions.commons_lang3}" |
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.
Both of these are needed for the Apache commons StringSubstitutor
being used for replacing the placeholder.
public static <T> String toJson(T value, String field) { | ||
try { | ||
XContentBuilder builder = JsonXContent.contentBuilder(); | ||
builder.value(value); |
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 handles the quoting for us, for example a string will be surrounded by quotes.
Pinging @elastic/ml-core (Team:ML) |
…tner/elasticsearch into ml-custom-model-placeholder
} | ||
|
||
private static void ensureNoMorePlaceholdersExist(String substitutedString, String field) { | ||
Matcher matcher = VARIABLE_PLACEHOLDER_PATTERN.matcher(substitutedString); |
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.
Wouldn't this just be substitutedString.indexOf('$') != -1
? Or do we allow instances of $
(like at the end of a string)?
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.
Yeah we'd have to look for ${
I suppose, but yeah I think the idea would be to let a string have any number of $
throughout it or even ${
(as long as it didn't have the closing }
).
…lastic#127271) * Adding placeholder functionality for request logic * Updating comments * Fixing tests * Adding missing licenses * Fixing String.format
💚 Backport successful
|
This PR adds logic to serialize java objects into JSON values and implement the placeholder replacement logic. The Custom service was leveraging
gson
to handle the serialization into json. This implementation uses our internalXContent
provider instead.This is needed for the Custom service functionality in the inference API. The Custom service will support PUT requests with placeholders (
${some_key}
). TheValidatingSubstitutor
handles replacing the placeholders with the values passed in via a map.Example PUT Request
In this example
${input_type}
would be replaced withquery
and${return_token}
would be replaced withtrue
.