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

Make data depend on security and task manager #128730

Merged
merged 3 commits into from
Mar 31, 2022
Merged

Make data depend on security and task manager #128730

merged 3 commits into from
Mar 31, 2022

Conversation

Dosant
Copy link
Contributor

@Dosant Dosant commented Mar 29, 2022 •
edited
Loading

Summary

close #123720
This pr is preparation for removing data_enhanced plugin #119321 . It adds needed dependencies for data plugin and fixes resulting circular dependencies

Previously removing data_enhanced was blocked by the issue that adding data -> security dependency caused CI flakiness, but recent server performance improvements in main resolved the issue.

@Dosant Dosant force-pushed the d/2022-03-29-revert-custom-integrations-fix branch from c92ec33 to 37cb8b2 Compare March 29, 2022 14:55
@Dosant Dosant changed the title D/2022 03 29 revert custom integrations fix Make data depend on security Mar 30, 2022
@Dosant Dosant changed the title Make data depend on security Make data depend on security and task manager Mar 30, 2022
Dosant
Dosant commented Mar 30, 2022
View reviewed changes
src/plugins/custom_integrations/kibana.json
],
"extraPublicDirs": ["common"],
"requiredPlugins": [],
"requiredBundles": ["presentationUtil"],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically there is still a circular dependency through requiredBundles that should be fixed. But it isn't blocking.

data -> security -> home -> customIntegrations -> presentationUtils -> data. I think we can break up customIntegrations -> presentationUtils

Issue: #121951

@Dosant Dosant added Team:AppServicesSv release_note:skip Skip the PR/issue when compiling release notes v8.3.0 labels Mar 30, 2022
@Dosant Dosant marked this pull request as ready for review March 30, 2022 09:33
@Dosant Dosant requested review from a team as code owners March 30, 2022 09:33
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServicesSv)

@Dosant
Copy link
Contributor Author

Dosant commented Mar 30, 2022

@elasticmachine merge upstream

@azasypkin
Copy link
Member

ACK: will review today

@azasypkin azasypkin self-requested a review March 30, 2022 11:24
azasypkin
azasypkin approved these changes Mar 30, 2022
View reviewed changes
Copy link
Member

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

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

Changes in Security plugin LGTM, tested Edit Role view (the only view in Security that relies on dataViews) locally and everything worked as expected.

ppisljar
ppisljar approved these changes Mar 30, 2022
View reviewed changes
Copy link
Member

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

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

code LGTM

@Dosant
Copy link
Contributor Author

Dosant commented Mar 31, 2022

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Page load bundle

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

id before after diff
security 50.2KB 50.2KB -5.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
customIntegrations 2 3 +1

Total ESLint disabled count

id before after diff
customIntegrations 2 3 +1

History

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

@Dosant Dosant merged commit a2ee23d into elastic:main Mar 31, 2022
@Dosant Dosant mentioned this pull request Mar 31, 2022
Closed
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 128730 or prevent reminders by adding the backport:skip label.

@kibanamachine kibanamachine added backport missing Added to PRs automatically when the are determined to be missing a backport. backport:skip This commit does not require backporting and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Apr 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ppisljar ppisljar ppisljar approved these changes

@azasypkin azasypkin azasypkin approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes v8.3.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

data dependent on security breaks tests
6 participants
@Dosant @elasticmachine @azasypkin @kibana-ci @kibanamachine @ppisljar