-
Notifications
You must be signed in to change notification settings - Fork 25.6k
[ML] Adding headers support for OpenAI chat completion #134504
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 headers support for OpenAI chat completion #134504
Conversation
|
Hi @jonathan-buttner, I've created a changelog YAML for you. |
|
|
||
| public void testIsEmpty() { | ||
| var randomSettings = new OpenAiChatCompletionTaskSettings(randomBoolean() ? null : "username"); | ||
| var randomSettings = new OpenAiEmbeddingsTaskSettings(randomBoolean() ? null : "username"); |
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.
Randomly found that this was testing the wrong class so switch it to the appropriate one.
…icsearch into ml-openai-headers
| ValidationException validationException = new ValidationException(); | ||
|
|
||
| String user = extractOptionalString(map, USER, ModelConfigurations.TASK_SETTINGS, validationException); | ||
| Map<String, Object> headers = extractOptionalMapRemoveNulls(map, HEADERS, validationException); |
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.
Since we need to add the headers to each task type I'll move this functionality to an abstract class in a later PR.
|
Pinging @elastic/ml-core (Team:ML) |
| } | ||
|
|
||
| /** | ||
| * Read an optional {@link Map} using the given key and value readers. The return Map is immutable. |
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.
Minor typo, should be "the returned Map is immutable"
|
|
||
| public void testFromMap_ReturnsUser() { | ||
| var settings = OpenAiChatCompletionRequestTaskSettings.fromMap(new HashMap<>(Map.of(OpenAiServiceFields.USER, "user"))); | ||
| public void testFromMap_ParsesCorrectly() { |
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.
It would be good to also have a test for the case where the headers map is null, since that's new functionality in this PR.
…icsearch into ml-openai-headers
* Adding headers support for openai chat completion and completion * Update docs/changelog/134504.yaml * [CI] Auto commit changes from spotless * Adjusting test name * Updating the changelog * Adding headers to the configuration * [CI] Auto commit changes from spotless * [CI] Update transport version definitions * Fixing transport versions --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>
* Adding headers support for openai chat completion and completion * Update docs/changelog/134504.yaml * [CI] Auto commit changes from spotless * Adjusting test name * Updating the changelog * Adding headers to the configuration * [CI] Auto commit changes from spotless * [CI] Update transport version definitions * Fixing transport versions --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>
This PR adds support for including custom headers in the chat completion and completion openai request. We'll need the support for other openai task types but I'll do that in a follow up PR to keep this one small.
The headers are stored in the
task_settings.This PR also introduce a new type for the
_inference/_servicesconfiguration results calledmapto represent the headers field.Example create inference API request
OpenAI configuration