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] Fixing capabilities when ML is disabled in elasticsearch #143622

Merged

Conversation

jgowdyelastic
Copy link
Member

@jgowdyelastic jgowdyelastic commented Oct 19, 2022

Moves the initialisation of ML saved objects and the auto sync task to after a license check has been performed. If ML is not enabled or the license is not platinum or trial we do not initialise the saved objects or create the auto sync task.

Updates the license checks to react to license changes. If the license changes from full (platinum or trial) to something else (e.g. basic) we disable the auto sync tasks.
If the license changes from non-full to full we initialise the saved objects and start the task.

Removes the canAccessMl capability in favour of explicit capabilities checks. canAccessMl was badly named and as a result was being misused by a few plugins, thinking it was the correct capability to check to see if ML is available, when really it was the very minimum check to cover our basic licensed features.

@jgowdyelastic jgowdyelastic self-assigned this Dec 12, 2022
@jgowdyelastic jgowdyelastic added non-issue Indicates to automation that a pull request should not appear in the release notes :ml release_note:skip Skip the PR/issue when compiling release notes v8.7.0 labels Dec 12, 2022
@jgowdyelastic jgowdyelastic marked this pull request as ready for review December 12, 2022 14:40
@jgowdyelastic jgowdyelastic requested a review from a team December 12, 2022 14:40
@jgowdyelastic jgowdyelastic requested a review from a team as a code owner December 12, 2022 14:40
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

Copy link
Contributor

@TattdCodeMonkey TattdCodeMonkey left a comment

Choose a reason for hiding this comment

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

enterprise-search changes LGTM.

const { hasPlatinumLicense } = useValues(LicensingLogic);
const hasMLPermissions = capabilities?.ml?.canAccessML ?? false;

if (!hasMLPermissions || !hasPlatinumLicense) {
Copy link
Member Author

@jgowdyelastic jgowdyelastic Dec 12, 2022

Choose a reason for hiding this comment

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

there is no need to test for the license level as this has already been done when setting the canGetTrainedModels capability.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
enterpriseSearch 2.0MB 2.0MB -44.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
ml 42.2KB 42.1KB -77.0B
Unknown metric groups

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
fleet 60 66 +6
osquery 109 115 +6
securitySolution 445 451 +6
total +20

References to deprecated APIs

id before after diff
ml 249 248 -1

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
fleet 69 75 +6
osquery 110 117 +7
securitySolution 521 527 +6
total +21

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @jgowdyelastic

Copy link
Contributor

@darnautov darnautov left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Tested and LGTM

@jgowdyelastic jgowdyelastic merged commit 1d6bac7 into elastic:main Dec 13, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Dec 13, 2022
@jgowdyelastic jgowdyelastic deleted the fixing-capabilities-when-mlis-disabled branch December 13, 2022 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting :ml non-issue Indicates to automation that a pull request should not appear in the release notes release_note:skip Skip the PR/issue when compiling release notes v8.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants