-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Index Patterns] Move rollup config to index pattern management v2 #102285
Conversation
@elasticmachine merge upstream |
Pinging @elastic/kibana-app-services (Team:AppServices) |
There was a problem hiding this 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(() => { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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(() => { |
There was a problem hiding this comment.
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
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Public APIs missing exports
Page load bundle
Unknown metric groupsAPI count
References to deprecated APIs
History
To update your PR or re-run it, just comment with: |
…lastic#102285) * move rollup config to index pattern management
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
…102285) (#102442) * move rollup config to index pattern management Co-authored-by: Matthew Kime <[email protected]>
…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
Summary
Changes to #102145 (which was reverted)
TLDR; Removing
rollup
plugin's dependence on theindexPatternManagment
plugin will allow the (in progress)indexPatternEditor
plugin to determine if therollup
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.