[ML] Adding fields for Inference service configuration API #121103
Conversation
jonathan-buttner
added
>refactoring
:ml
| new SettingsConfiguration.Builder(supportedTaskTypes).setDescription("") | ||
| .setLabel("HTTP Schema") | ||
| .setRequired(true) | ||
| .setRequired(false) |
There was a problem hiding this comment.
This doesn't need to be required because it's defaulted for all task types. For example:
| () -> { | ||
| var configurationMap = new HashMap<String, SettingsConfiguration>(); | ||
|
|
||
| configurationMap.put( |
There was a problem hiding this comment.
It's optional for the v1 endpoints for cohere: https://docs.cohere.com/v1/reference/embed#request.body.model
It's required for v2 endpoints for cohere. We use v1 right now.
There was a problem hiding this comment.
model
string
Required
Defaults to embed-english-v2.0
required but with a default? 🧐
| "The name of the model to use for the inference task." | ||
| ) | ||
| .setLabel("Model ID") | ||
| .setRequired(true) |
There was a problem hiding this comment.
Required for both text embedding and rerank: https://api.jina.ai/redoc#tag/embeddings
| ); | ||
|
|
||
| configurationMap.put( | ||
| URL, |
There was a problem hiding this comment.
I don't think we want to expose the URL for users. This was meant to be an internal way to test. Although I know we've leveraged that to connect to other providers that adhere to the OpenAI spec. So If we want to keep this I can understand the argument for that. We should make it optional though. Let me know what you think.
There was a problem hiding this comment.
URL is configurable in the API so that it can be used with a LocalAI server https://www.elastic.co/search-labs/blog/localai-for-text-embeddings
It shouldn't be configurable in the UI though as it makes little sense to change this when creating up an OpenAI endpoint
|
Pinging @elastic/ml-core (Team:ML) |
| () -> { | ||
| var configurationMap = new HashMap<String, SettingsConfiguration>(); | ||
|
|
||
| configurationMap.put( |
There was a problem hiding this comment.
model
string
Required
Defaults to embed-english-v2.0
required but with a default? 🧐
| ); | ||
|
|
||
| configurationMap.put( | ||
| URL, |
There was a problem hiding this comment.
URL is configurable in the API so that it can be used with a LocalAI server https://www.elastic.co/search-labs/blog/localai-for-text-embeddings
It shouldn't be configurable in the UI though as it makes little sense to change this when creating up an OpenAI endpoint
…21103) * Adding fields and making some optional * Fixing tests
…21103) * Adding fields and making some optional * Fixing tests
…21103) * Adding fields and making some optional * Fixing tests
4 participants
This PR adds a few more fields that can be returned to the UI and cleans up a few things. It is a bug that the fields weren't included initially.
There are a few fields that should be switched to required or optional that I addressed as well.