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.

Sign up for GitHub

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

Jump to bottom

decouple anomaly job creation action from Embeddable framework #176869

Merged
merged 26 commits into from
Mar 11, 2024
Merged

decouple anomaly job creation action from Embeddable framework #176869

merged 26 commits into from
Mar 11, 2024

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Feb 13, 2024 •
edited
Loading

part of #175138

PR decouples createMLADJobAction action from Embeddable framework. This means that instead of reading values from embeddable.getInput(), values are read from presentation publishing interfaces. Existing embeddables expose both Embeddable and presentation publishing interfaces so they work with both. In the future, as embeddables get refactored to the new embeddable system, then they will only expose presentation publishing interfaces. Migrating away from old interfaces so that refactoring embeddables can start.

@nreese
Copy link
Contributor Author

nreese commented Feb 15, 2024

/ci

@nreese
Copy link
Contributor Author

nreese commented Feb 15, 2024

/ci

@nreese
Copy link
Contributor Author

nreese commented Feb 15, 2024

/ci

@nreese nreese added the blocked label Feb 15, 2024
@nreese nreese mentioned this pull request Feb 15, 2024
Merged
@nreese
Copy link
Contributor Author

nreese commented Feb 15, 2024

/ci

@nreese
Copy link
Contributor Author

nreese commented Feb 15, 2024

/ci

@nreese nreese marked this pull request as ready for review February 15, 2024 22:03
@nreese nreese requested review from a team as code owners February 15, 2024 22:03
@nreese nreese added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas release_note:skip Skip the PR/issue when compiling release notes Team:ML Team label for ML (also use :ml) and removed blocked labels Feb 15, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@nreese nreese added the v8.14.0 label Feb 15, 2024
@nreese
Copy link
Contributor Author

nreese commented Feb 20, 2024

@elasticmachine merge upstream

nreese added a commit that referenced this pull request Feb 20, 2024
@nreese @kibanamachine
[lens] update LensEmbeddable.getSavedVis to return LensSavedObjectAtt…
82d7244 
…ributes (#177064)

Blocking #176869

While working on #176869, I
discovered there is a type mismatch between `LensEmbeddable.getSavedVis`
internal implementation and consumers of `LensEmbeddable.getSavedVis`
results.

LensEmbeddable.getSavedVis returns `Readonly<Document | undefined>`.
LensEmbeddable.getSavedVis consumers are typing the response as
`LensSavedObjectAttributes`.

This PR updates `LensEmbeddable.getSavedVis` to return
`Readonly<LensSavedObjectAttributes | undefined>` since that is how its
being used.

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@alvarezmelissa87 alvarezmelissa87 self-requested a review February 21, 2024 15:53
@Heenawter Heenawter mentioned this pull request Feb 22, 2024
Closed
fkanout pushed a commit to fkanout/kibana that referenced this pull request Mar 4, 2024
@nreese @kibanamachine @fkanout
[lens] update LensEmbeddable.getSavedVis to return LensSavedObjectAtt…
6678134 
…ributes (elastic#177064)

Blocking elastic#176869

While working on elastic#176869, I
discovered there is a type mismatch between `LensEmbeddable.getSavedVis`
internal implementation and consumers of `LensEmbeddable.getSavedVis`
results.

LensEmbeddable.getSavedVis returns `Readonly<Document | undefined>`.
LensEmbeddable.getSavedVis consumers are typing the response as
`LensSavedObjectAttributes`.

This PR updates `LensEmbeddable.getSavedVis` to return
`Readonly<LensSavedObjectAttributes | undefined>` since that is how its
being used.

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@nreese
Copy link
Contributor Author

nreese commented Mar 5, 2024

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

merge conflict between base and head

@nreese nreese requested a review from a team as a code owner March 5, 2024 13:48
@nreese nreese requested a review from ThomThomson March 5, 2024 13:48
@nreese
Copy link
Contributor Author

nreese commented Mar 5, 2024

@elasticmachine merge upstream

ThomThomson
ThomThomson approved these changes Mar 8, 2024
View reviewed changes
Copy link
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

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

Looks like all of my comments have been addressed!

Left one nit, but it's not a blocker. Eventually I'd very much like to have one full source of truth for what constitutes a dashboard API. Same with Lens and Maps.

src/plugins/dashboard/public/dashboard_api.ts Outdated Show resolved Hide resolved
drewdaemon
drewdaemon approved these changes Mar 8, 2024
View reviewed changes
Copy link
Contributor

@drewdaemon drewdaemon left a comment

Choose a reason for hiding this comment

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

Cool

@nreese
Copy link
Contributor Author

nreese commented Mar 11, 2024

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
maps 1147 1148 +1
ml 1928 1950 +22
total +23

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
maps 260 263 +3

Async chunks

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

id before after diff
lens 1.4MB 1.4MB -7.0B
maps 2.9MB 2.9MB -7.0B
ml 3.7MB 3.7MB +4.3KB
total +4.3KB

Page load bundle

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

id before after diff
lens 45.5KB 45.9KB +335.0B
maps 49.1KB 49.6KB +458.0B
total +793.0B
Unknown metric groups

API count

id before after diff
maps 261 264 +3

History

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

cc @nreese

@nreese nreese merged commit d7d2e84 into elastic:main Mar 11, 2024
16 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ThomThomson ThomThomson ThomThomson approved these changes

@alvarezmelissa87 alvarezmelissa87 alvarezmelissa87 approved these changes

@drewdaemon drewdaemon drewdaemon approved these changes

Assignees

@nreese nreese

Labels
backport:skip This commit does not require backporting project:embeddableRebuild release_note:skip Skip the PR/issue when compiling release notes Team:ML Team label for ML (also use :ml) Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.14.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@nreese @elasticmachine @kibanamachine @kibana-ci @drewdaemon @alvarezmelissa87 @ThomThomson