-
Notifications
You must be signed in to change notification settings - Fork 25.1k
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] Refactor OpenAI to use ConstructingObjectParser and consolidate class locations #124380
base: main
Are you sure you want to change the base?
[ML] Refactor OpenAI to use ConstructingObjectParser and consolidate class locations #124380
Conversation
ensureExpectedToken(XContentParser.Token.START_OBJECT, token, jsonParser); | ||
|
||
positionParserAtTokenAfterField(jsonParser, "choices", FAILED_TO_FIND_FIELD_TEMPLATE); | ||
try (var p = XContentFactory.xContent(XContentType.JSON).createParser(XContentParserConfiguration.EMPTY, response.body())) { |
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.
The major changes here are to use the ConstructingObjectParser
instead of iterating by token.
@@ -0,0 +1,107 @@ | |||
/* |
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 class was moved and transitioned to use the ConstructingObjectParser
.
@@ -1,110 +0,0 @@ | |||
/* |
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 was moved to external.openai
and transitioned to use the ConstructingObjectParser
.
@@ -219,7 +219,7 @@ public void testCreate_AzureOpenAiEmbeddingsModel_FailsFromInvalidResponseFormat | |||
PlainActionFuture<InferenceServiceResults> listener = new PlainActionFuture<>(); | |||
action.execute(new DocumentsOnlyInput(List.of("abc")), InferenceAction.Request.DEFAULT_TIMEOUT, listener); | |||
|
|||
var failureCauseMessage = "Failed to find required field [data] in OpenAI embeddings response"; | |||
var failureCauseMessage = "Required [data]"; |
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.
If we encounter a parsing failure where a field is missing, the error message will be decorated on the way back in one of the upstream listeners. The error message will have the openai information included elsewhere.
@@ -533,7 +533,7 @@ public void testCreate_OpenAiChatCompletionModel_FailsFromInvalidResponseFormat( | |||
PlainActionFuture<InferenceServiceResults> listener = new PlainActionFuture<>(); | |||
action.execute(new ChatCompletionInput(List.of("abc")), InferenceAction.Request.DEFAULT_TIMEOUT, listener); | |||
|
|||
var failureCauseMessage = "Failed to find required field [choices] in OpenAI chat completions response"; | |||
var failureCauseMessage = "Required [choices]"; | |||
var thrownException = expectThrows(ElasticsearchStatusException.class, () -> listener.actionGet(TIMEOUT)); | |||
assertThat( | |||
thrownException.getMessage(), |
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.
Here's an example of the message containing openai and chat completions information already.
Pinging @elastic/ml-core (Team:ML) |
This PR refactors the OpenAI response parsing logic.
external.openai