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

[Index Patterns] Move rollup config to index pattern management v2 #102285

Merged

Conversation

mattkime
Copy link
Contributor

@mattkime mattkime commented Jun 16, 2021

Summary

Changes to #102145 (which was reverted)

  • Move uiSettings check from setup lifecycle function to when the UI is rendered - d74614a
  • Check whether the uiSetting value is provided before getting the value to avoid errors - 217e304

TLDR; Removing rollup plugin's dependence on the indexPatternManagment plugin will allow the (in progress) indexPatternEditor plugin to determine if the rollup plugin is active, otherwise there's a circular dependency. This is also tech debt reduction.

Refactor index pattern creation code so its all within the index pattern management plugin. Previously rollup related code was in the rollup plugin since it was an x-pack feature. We're no longer creating one off APIs to maintain the distinction between x-pack features and OSS code.

@mattkime mattkime added auto-backport Deprecated - use backport:version if exact versions are needed Feature:Data Views Data Views code and UI - index patterns before 8.0 Feature:Rollups release_note:skip Skip the PR/issue when compiling release notes Team:AppServices v7.14.0 v8.0.0 labels Jun 16, 2021
@mattkime
Copy link
Contributor Author

@elasticmachine merge upstream

@mattkime mattkime marked this pull request as ready for review June 16, 2021 20:50
@mattkime mattkime requested a review from a team as a code owner June 16, 2021 20:50
@elasticmachine
Copy link
Contributor

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

@mattkime mattkime requested a review from cjcenizal June 16, 2021 20:50
@mattkime mattkime changed the title Rollup creation code to index pattern mgmt [Index Patterns] Move rollup config to index pattern management v2 Jun 16, 2021
Copy link
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

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

Haven't tested locally but code LGTM! Had a couple questions/suggestions but nothing blocking.

const config = new Config({ httpClient });
export class IndexPatternCreationManager {
start({ httpClient, uiSettings }: IndexPatternCreationManagerStart) {
const getConfigs = memoize(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you help me understand why this is memoized? The provided function doesn't accept any arguments and we're not passing a second resolver argument to memoize, so I don't see how the memoize function could use any map cache keys to memoize the results of the provided function.

Maybe it makes more sense to use once instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, that makes sense

const config = new Config();
export class IndexPatternListManager {
start({ uiSettings }: IndexPatternListManagerStart) {
const getConfigs = memoize(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

…om:mattkime/kibana into rollup_creation_code_to_index_pattern_mgmt
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
indexPatternManagement 162 167 +5
rollup 169 165 -4
total +1

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
indexPatternManagement 45 0 -45

Async chunks

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

id before after diff
rollup 230.2KB 230.2KB +9.0B

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
indexPatternManagement 3 0 -3

Page load bundle

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

id before after diff
indexPatternManagement 12.9KB 19.1KB +6.2KB
rollup 40.8KB 33.8KB -7.0KB
total -888.0B
Unknown metric groups

API count

id before after diff
indexPatternManagement 45 0 -45

References to deprecated APIs

id before after diff
indexPatternManagement 68 0 -68

History

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

@mattkime mattkime merged commit bfca0c3 into elastic:master Jun 17, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jun 17, 2021
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Jun 17, 2021
…102285) (#102442)

* move rollup config to index pattern management

Co-authored-by: Matthew Kime <matt@mattki.me>
jloleysens added a commit to jloleysens/kibana that referenced this pull request Jun 17, 2021
…egrations-to-global-search

* 'master' of github.com:elastic/kibana: (46 commits)
  [Lens] Add some more documentation for dynamic coloring (elastic#101369)
  hide not searchable results when no term (elastic#102401)
  [Lens] Fix Formula functional test with multiple suggestions (elastic#102378)
  Fix trusted apps modified by field displayed as a date field (elastic#102377)
  [Lens] Docs for time shift (elastic#102048)
  update readme of logs-metrics-ui (elastic#101968)
  Refactor observability plugin breadcrumbs (elastic#102290)
  [Index Patterns] Move rollup config to index pattern management v2 (elastic#102285)
  [Security Solution][Endpoint] Isolate Action should only be available to Platinum+ licenses (elastic#102374)
  [build] Updates Ironbank templates (elastic#102407)
  Update security best practices document (elastic#100814)
  [Enterprise Search] Set up initial KibanaPageTemplate  (elastic#102170)
  [Reporting/Docs] Add section to troubleshooting guide to explain the StatusCodeError logs (elastic#102278)
  [DOCS] Updating Elastic Security Overview topic  (elastic#101922)
  [Uptime] refactor Synthetics Integration package UI (elastic#102080)
  [Task Manager] Log at different levels based on the state (elastic#101751)
  [APM] Fixing time comparison types (elastic#101423)
  [RAC] Update alert documents in lifecycle rule type helper (elastic#101598)
  [ML] Functional tests - fix and re-activate alerting flyout test (elastic#102368)
  [Reporting] remove unused reference to path.data config (elastic#102267)
  ...

# Conflicts:
#	x-pack/plugins/fleet/kibana.json
majagrubic pushed a commit to majagrubic/kibana that referenced this pull request Jun 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Data Views Data Views code and UI - index patterns before 8.0 Feature:Rollups release_note:skip Skip the PR/issue when compiling release notes v7.14.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants