[ML] Require basic licence for the Elastic Inference Service #137434
Conversation
# Conflicts: # x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/action/BaseTransportInferenceAction.java # x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/action/filter/ShardBulkInferenceActionFilter.java
|
Pinging @elastic/ml-core (Team:ML) |
|
Hi @davidkyle, I've created a changelog YAML for you. |
benwtrent
left a comment
benwtrent
left a comment
There was a problem hiding this comment.
easy peasy. It makes sense to handle all the license checks as normal, but pivoting on type
| if (InferenceLicenceCheck.isServiceLicenced(inferenceProvider.service.name(), licenseState) == false) { | ||
| try (onFinish) { | ||
| for (FieldInferenceRequest request : requests) { | ||
| addInferenceResponseFailure( | ||
| request.bulkItemIndex, | ||
| InferenceLicenceCheck.complianceException(inferenceProvider.service.name()) | ||
| ); | ||
| } | ||
| return; | ||
| } | ||
| } |
There was a problem hiding this comment.
@carlosdelest what do you think of this? It seems OK to me. Both the old and new license check failures are per bulk item request. Now we just delay it until we have the inference provider loaded.
There was a problem hiding this comment.
That makes sense - we're checking the inference provider service when we have it. As requests are grouped by inference provider, we can fail all of them at once in case it's not compliant. 👍
|
|
||
| public static final LicensedFeature.Momentary EIS_INFERENCE_FEATURE = LicensedFeature.momentary( | ||
| "inference", | ||
| "eis", |
There was a problem hiding this comment.
Its a bit confusing to me, the ElasticInferenceService.NAME is elastic, but our license service name is eis. So the user configures elastic to get the license with name eis.
Could we name this elastic to match the name of the ElasticInferenceService.NAME?
There was a problem hiding this comment.
I've changed this to the proper name "Elastic Inference Service"
The error the user sees if the licence is not compatible is either:
"current license is non-compliant for [inference]"
Or
"current license is non-compliant for [Elastic Inference Service]"
jonathan-buttner
left a comment
jonathan-buttner
left a comment
There was a problem hiding this comment.
Looks good, I agree with Ben, could we have license string be elastic instead of eis? Or maybe Elastic Inference Service?
| return; | ||
| } | ||
|
|
||
| if (InferenceLicenceCheck.isServiceLicenced(serviceName, licenseState) == false) { |
There was a problem hiding this comment.
nit: I wonder if we should move the reserved ID check to below this check?
# Conflicts: # x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/action/TransportPutInferenceModelAction.java
5 participants
This change allows different services in the Inference API to be licensed at different levels by making the licence check per service. EIS is licensed at the basic level, everything else remains enterprise.