Skip to content

[ML] Removing global isServerless flag#166651

Merged
jgowdyelastic merged 47 commits intoelastic:mainfrom
jgowdyelastic:removing-global-isServerless-flag
Sep 27, 2023
Merged

[ML] Removing global isServerless flag#166651
jgowdyelastic merged 47 commits intoelastic:mainfrom
jgowdyelastic:removing-global-isServerless-flag

Conversation

@jgowdyelastic
Copy link
Copy Markdown
Member

@jgowdyelastic jgowdyelastic commented Sep 18, 2023 •
edited
Loading

Removed isServerless flag which lived in our global context and had to be passed about to the various components which create their own version of the context using getMlGlobalServices
This PR adds a new context which contains flags for all of the features which can be toggled when in serverless mode.
Flags added:

 showNodeInfo
 showMLNavMenu
 showLicenseInfo
 isADEnabled
 isDFAEnabled
 isNLPEnabled

The enabled features flags are now read from the config file client side, rather than using capabilities.

Additional changes:

  • Changes the wording of the awaiting ML node callout in serverless.
  • In the search project, the default ML page is the trained models list and not Overview
  • Reenables the Memory Usage page for all projects

@jgowdyelastic jgowdyelastic mentioned this pull request Sep 19, 2023
Merged
9 tasks
@jgowdyelastic jgowdyelastic self-assigned this Sep 19, 2023
@jgowdyelastic jgowdyelastic mentioned this pull request Sep 20, 2023
Merged
@jgowdyelastic jgowdyelastic marked this pull request as ready for review September 21, 2023 14:14
@jgowdyelastic jgowdyelastic requested a review from a team as a code owner September 21, 2023 14:14
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Sep 21, 2023

Pinging @elastic/ml-ui (:ml)

@kc13greiner kc13greiner self-requested a review September 22, 2023 12:15
kc13greiner
kc13greiner approved these changes Sep 22, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@kc13greiner kc13greiner left a comment

Choose a reason for hiding this comment

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

LGTM!

qn895
qn895 reviewed Sep 25, 2023
View reviewed changes
x-pack/packages/ml/date_picker/src/components/full_time_range_selector.tsx Outdated
</EuiButton>
</EuiToolTip>
{isServerless || hideFrozenDataTierChoice ? null : (
{showFrozenDataTierChoice && showFrozenDataTierChoiceProp ? (
Copy link
Copy Markdown
Member

@qn895 qn895 Sep 25, 2023

Choose a reason for hiding this comment

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

What happens when showFrozenDataTierChoice from useDatePickerContext is undefined but showFrozenDataTierChoiceProp is explicity set to true? If both have to be true, might be good to document that in the FullTimeRangeSelectorProps.

Copy link
Copy Markdown
Member Author

@jgowdyelastic jgowdyelastic Sep 25, 2023

Choose a reason for hiding this comment

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

This is a mistake introduced by me switching from "hide" to "show" in the variable names.
Looking at this again though, I don't think the prop is needed at all anymore. Everywhere this component is used it's wrapped in the data picker context.

Copy link
Copy Markdown
Member Author

@jgowdyelastic jgowdyelastic Sep 25, 2023

Choose a reason for hiding this comment

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

I've removed the prop here 32a833f

@qn895
Copy link
Copy Markdown
Member

qn895 commented Sep 25, 2023

Tested latest changes and LGTM 🎉

peteharverson
peteharverson reviewed Sep 26, 2023
View reviewed changes
x-pack/plugins/ml/public/application/memory_usage/memory_usage_page.tsx
useTimefilter({ timeRangeSelector: false, autoRefreshSelector: true });

const isServerless = useIsServerless();
const { showNodeInfo } = useEnabledFeatures();
Copy link
Copy Markdown
Contributor

@peteharverson peteharverson Sep 26, 2023

Choose a reason for hiding this comment

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

In the Observability project, the Memory Usage page is failing to load, with a 404 error:

image

Copy link
Copy Markdown
Member Author

@jgowdyelastic jgowdyelastic Sep 26, 2023

Choose a reason for hiding this comment

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

Fixed in 14e1f50
I had incorrectly moved the model management route registration behind a check for dfa and nlp features, thinking the routes are for trained model management only.
However these routes are for AD models too.

peteharverson
peteharverson approved these changes Sep 26, 2023
View reviewed changes
Copy link
Copy Markdown
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.

Latest changes LGTM

@kibana-ci
Copy link
Copy Markdown

kibana-ci commented Sep 27, 2023 •
edited
Loading

💛 Build succeeded, but was flaky

  • Buildkite Build
  • Commit: 542ffb3
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-166651-542ffb3a2128

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #3 / maps app embeddable maps add-to-dashboard save flow should allow existing maps be added by reference to an existing dashboard

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
aiops 593.9KB 594.0KB +85.0B
dataVisualizer 613.3KB 613.3KB -2.0B
ml 3.5MB 3.5MB +1.5KB
transform 401.1KB 401.1KB -26.0B
total +1.5KB

Canvas Sharable Runtime

The Canvas "shareable runtime" is an bundle produced to enable running Canvas workpads outside of Kibana. This bundle is included in third-party webpages that embed canvas and therefor should be as slim as possible.

id before after diff
module count - 5601 +5601
total size - 5.9MB +5.9MB

Page load bundle

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

id before after diff
ml 75.6KB 75.9KB +316.0B
Unknown metric groups

API count

id before after diff
@kbn/ml-date-picker 49 48 -1

History

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

cc @jgowdyelastic

@jgowdyelastic jgowdyelastic merged commit c18513e into elastic:main Sep 27, 2023
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label Sep 27, 2023
@jgowdyelastic jgowdyelastic deleted the removing-global-isServerless-flag branch September 27, 2023 08:32
@jgowdyelastic jgowdyelastic mentioned this pull request Sep 27, 2023
Merged
jgowdyelastic added a commit that referenced this pull request Sep 28, 2023
@jgowdyelastic
[Transforms] Fixing use full data button (#167404)
046718c 
Fixing bug introduced by a merge clash between
#166622 and
#166651

The `hideFrozenDataTierChoice` prop is no longer available in the
`FullTimeRangeSelector` component and so the transforms plugin needs to
set `showFrozenDataTierChoice` in the data picker context.
@watson watson added the Project:Serverless Work as part of the Serverless project for its initial release label Oct 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@peteharverson peteharverson peteharverson approved these changes

@kc13greiner kc13greiner kc13greiner approved these changes

@qn895 qn895 qn895 approved these changes

Assignees

@jgowdyelastic jgowdyelastic

Labels

backport:skip This PR does not require backporting ci:build-serverless-image :ml Project:Serverless Work as part of the Serverless project for its initial release release_note:skip Skip the PR/issue when compiling release notes v8.11.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@jgowdyelastic @elasticmachine @qn895 @kibana-ci @peteharverson @kc13greiner @watson @kibanamachine