Skip to content
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] simplifying model license checks #80031

Conversation

benwtrent
Copy link
Member

Since trained models require either a platinum or basic license, our license checking can be
simplified.

Now, we check if ML APIs are allowed (platinum) or if the trained model is basic.

relates to: #79908

@elasticmachine elasticmachine added the Team:ML Meta label for the ML team label Oct 28, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core (Team:ML)

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One suggestion

@@ -268,7 +268,7 @@ public InferencePipelineAggregationBuilder rewrite(QueryRewriteContext context)
loadedModel.set(model);

boolean isLicensed = MachineLearningField.ML_API_FEATURE.check(licenseState)
|| featureCheckForMode(model.getLicenseLevel(), licenseState);
|| model.getLicenseLevel() == License.OperationMode.BASIC;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the order wrt the ML check above be reversed? That way if it is basic we don't trigger tracking for ML use?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rjernst 100%, that makes sense as we wouldn't want to track platinum usage. I will make that change. Good catch.

@benwtrent benwtrent added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Oct 28, 2021
@elasticsearchmachine elasticsearchmachine merged commit f69faa7 into elastic:master Oct 28, 2021
@benwtrent benwtrent deleted the feature/ml-fixing-previously-license-checks-for-models branch October 28, 2021 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :ml Machine learning >non-issue Team:ML Meta label for the ML team v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants