[ML] Automatically download the ELSER model when PUT in _inference #104334
Conversation
|
Pinging @elastic/ml-core (Team:ML) |
|
Hi @maxhniebergall, I've created a changelog YAML for you. |
...e-service-tests/src/javaRestTest/java/org/elasticsearch/xpack/inference/InferenceCrudIT.java
Outdated
Show resolved
Hide resolved
| * @param modelVariant The configuration of the model variant to be downloaded | ||
| * @param listener The listener | ||
| */ | ||
| default void putModel(Model modelVariant, ActionListener<Boolean> listener) { |
There was a problem hiding this comment.
Should we leverage the start method that's in this interface for downloading the model instead of adding a new one?
There was a problem hiding this comment.
I think we could use start, although I am unsure about the benefit. I suppose it would decrease the complexity of the interface, and it is a bit weird that startModel in TransportPutInferenceModelAction calls two methods in the InferenceService. However, I think that this also represents the underlying business logic (that start is both starting an inference model and putting a trained model definition). I think that creating a second method in the interface has the benefit of splitting up these two objectives into separate methods in the ElserMlNodeService (and other InferenceServices), which represent the two API calls we are making to support the business logic.
So, I can't really imagine why it would be better to avoid creating another method in this interface, but please let me know if I missed it!
| var input = new TrainedModelInput(fieldNames); | ||
| var config = TrainedModelConfig.builder().setInput(input).setModelId(modelVariant).build(); | ||
| PutTrainedModelAction.Request putRequest = new PutTrainedModelAction.Request(config, false, true); | ||
| executeAsyncWithOrigin(client, ML_ORIGIN, PutTrainedModelAction.INSTANCE, putRequest, listener.delegateFailure((l, r) -> { |
There was a problem hiding this comment.
Hmm I wonder if we should be using a new origin for inference instead of ML_ORIGIN 🤔 @davidkyle ? Or do we need this as ML_ORIGIN so it gets the permissions correct?
There was a problem hiding this comment.
I didn't really consider using a different origin. I'll have to look into what the impact of the origin is. Thanks for pointing this out!
There was a problem hiding this comment.
Looks like there is already an inference origin, so I switched to using that. Thanks @jonathan-buttner !
…lastic#104334) * Automatically download ELSER when PUT in _inference * Revert "Disable elser download test case in inf IT (elastic#104271)" * add IT * disable IT
Closes https://github.com/elastic/ml-team/issues/1098
Previously, to be able to use the ELSER model, there were two steps: 1. Put (download) ELSER using the trained models API; 2. Put the ELSER model using the _inference API.
With this change, these two steps are combined, so now to install ELSER in _inference, all one has to do is PUT the ELSER model using the _inference API.
For example,