search

elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.35k stars 7.98k forks source link

[Discover] Migrate from `index` to `dataSource` in app state #182321

Closed davismcphee closed 3 days ago

davismcphee commented 2 weeks ago

Summary

This PR replaces the index property of Discover's app state with a new dataSource property, containing two initial types: dataView and esql. The new data source abstraction will be used in the One Discover data source context resolution process, as well as preparing Discover to support additional data sources as needed in the future. Additionally, it creates a clearer division in the Discover state between "data view mode" and "ES|QL mode", where previously this was implicit based on whether the query property was an ES|QL query vs KQL or Lucene.

The goal of this PR is to add initial support for the dataSource property without introducing broader changes to the Discover state management. However, these changes open up opportunities for future improvements to simplify the state management, such as checking the data source type to determine which mode Discover is in instead of always checking the query directly. These types of enhancements can be built on this foundation in followup PRs.

Part of #181963.

Checklist

For maintainers

davismcphee commented 2 weeks ago

/ci

davismcphee commented 1 week ago

/ci

davismcphee commented 1 week ago

/ci

davismcphee commented 1 week ago

/ci

davismcphee commented 1 week ago

/ci

davismcphee commented 1 week ago

/ci

elasticmachine commented 1 week ago

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

davismcphee commented 1 week ago

Thanks for taking a look @tonyghiani! The logic for handling the legacy index param is located here next to where we handle our other legacy params. I added some unit tests for it, and also some of the functional tests use the old URL format which provides some additional coverage that the handling it working too.

davismcphee commented 5 days ago

/ci

stratoula commented 5 days ago

@davismcphee one question. This means that saved urls with index are not going to work anymore? Or do we have a mechanism to change the "old" url to the new one? (I haven't tested)

kertal commented 5 days ago

@davismcphee one question. This means that saved urls with index are not going to work anymore? Or do we have a mechanism to change the "old" url to the new one? (I haven't tested)

@stratoula The answer can be found here https://github.com/elastic/kibana/pull/182321/files#r1598144877 It's definitely being migrated

kibana-ci commented 4 days ago

:yellow_heart: Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
discover 855 858 +3

Async chunks

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

id before after diff
discover 722.5KB 723.5KB +975.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
discover 27 28 +1

Page load bundle

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

id before after diff
discover 35.1KB 35.6KB +496.0B

History

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

cc @davismcphee