[Query] Use a minimal index pattern interface for es query #102364

lizozom · 2021-06-16T15:18:00Z

Summary

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately

…-json-utils

…into es-kuery/move-json-utils

…interface

lizozom · 2021-06-16T15:19:21Z

src/plugins/data/common/es_query/es_query/types.ts

+
+import { IFieldType } from '../../index_patterns';
+
+export interface MinimalIndexPattern {


@mattkime do you have a better name for this?
And what do you think in general?

I think this is fine. Its a bit hacky but it solves the problem without increasing complexity.

lukeelmers

Overall I think the approach here makes sense based on our conversation:

IIndexPattern stays as-is and extends from the base/minimal index pattern
The minimal interface in es_query makes it explicit exactly which pieces of index patterns es_query really depends on
It's simple

The main downside is that it feels awkward to have index patterns depend on es-query, but I don't think that's avoidable without duplicating the types.

lukeelmers · 2021-06-16T19:32:33Z

src/plugins/data/common/es_query/es_query/types.ts

+import { IFieldType } from '../../index_patterns';
+
+export interface MinimalIndexPattern {
+  fields: IFieldType[];


Isn't the dependency on IFieldType[] going to be a problem too, whenever this gets split out into a package? I'd imagine you'd need to relocate that as well.

FWIW, if it starts to feel weird having all of this stuff crammed in a @kbn/es-query package, someone had proposed creating something like @kbn/data-types as a type-specific package just for the data plugin, so that these things could live there instead (and be imported by @kbn/es-query)

Neither option feels great, just wanted to mention it as something to consider.

Yes, I intend to handle IFieldType in a followup PR, before moving both into a package

…interface

elasticmachine · 2021-06-23T13:59:27Z

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

mattkime

Nice work, changes look good!

lizozom · 2021-06-23T14:43:58Z

@elasticmachine merge upstream

…interface

…to es-kuery/new-interface

kibanamachine · 2021-06-23T17:34:18Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`data`	572	573	+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
`data`	3272	3265	-7

Async chunks

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

id	before	after	diff
`data`	165.4KB	165.4KB	-96.0B

Page load bundle

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

id	before	after	diff
`data`	832.8KB	832.4KB	-485.0B

Unknown metric groups

API count

id	before	after	diff
`data`	3837	3830	-7

History

💚 Build #133588 succeeded 308ec01
💛 Build #133480 was flaky 1bab47e
💔 Build #131767 failed cdb1f82
💔 Build #131730 failed 51c490b

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

cc @lizozom

kibanamachine · 2021-06-23T19:43:17Z

💔 Backport failed

Status	Branch	Result
❌	7.x	Commit could not be cherrypicked due to conflicts

To backport manually run:
node scripts/backport --pr 102364

kibanamachine · 2021-06-25T19:48:07Z

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

…02364) * Move JSON utils to utils package * Imports from tests * delete * split package * docs * test * test * imports * minimal index pattern * move some functions out and use miniaml ip in all es-kuery * docs * docs * rename Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> # Conflicts: # docs/development/plugins/data/public/kibana-plugin-plugins-data-public.iindexpattern.md # src/plugins/data/public/public.api.md

…103453) * Move JSON utils to utils package * Imports from tests * delete * split package * docs * test * test * imports * minimal index pattern * move some functions out and use miniaml ip in all es-kuery * docs * docs * rename Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> # Conflicts: # docs/development/plugins/data/public/kibana-plugin-plugins-data-public.iindexpattern.md # src/plugins/data/public/public.api.md

Liza K and others added 16 commits June 14, 2021 15:20

Move JSON utils to utils package

148af16

Imports from tests

0b79d06

Merge branch 'master' of github.com:elastic/kibana into es-kuery/move…

e5f9f80

…-json-utils

delete

83645da

Merge branch 'master' into es-kuery/move-json-utils

33f7a87

Merge branch 'master' of github.com:elastic/kibana into es-kuery/move…

6b51a9a

…-json-utils

split package

04206fb

Merge branch 'master' of github.com:elastic/kibana into es-kuery/move…

3f895f4

…-json-utils

Merge branch 'es-kuery/move-json-utils' of github.com:lizozom/kibana …

4248be7

…into es-kuery/move-json-utils

docs

b1ccf5c

test

e02bd98

test

9ab64da

imports

6347118

minimal index pattern

508ed0e

move some functions out and use miniaml ip in all es-kuery

25884bb

Merge branch 'master' of github.com:elastic/kibana into es-kuery/new-…

51c490b

…interface

lizozom commented Jun 16, 2021

View reviewed changes

lizozom requested review from mattkime and lukeelmers June 16, 2021 15:23

docs

cdb1f82

lukeelmers reviewed Jun 16, 2021

View reviewed changes

Liza K added 3 commits June 21, 2021 18:44

docs

c49630b

Merge branch 'master' of github.com:elastic/kibana into es-kuery/new-…

e611902

…interface

Merge branch 'master' of github.com:elastic/kibana into es-kuery/new-…

1bab47e

…interface

lizozom self-assigned this Jun 23, 2021

lizozom added auto-backport Deprecated - use backport:version if exact versions are needed v7.14.0 v8.0.0 labels Jun 23, 2021

lizozom marked this pull request as ready for review June 23, 2021 13:58

lizozom requested a review from a team as a code owner June 23, 2021 13:58

lizozom added release_note:skip Skip the PR/issue when compiling release notes Team:AppServices labels Jun 23, 2021

mattkime approved these changes Jun 23, 2021

View reviewed changes

lizozom mentioned this pull request Jun 23, 2021

[Data - Query Service] Move es-query back into a dedicated package #51659

Closed

3 tasks

lukeelmers approved these changes Jun 23, 2021

View reviewed changes

kibanamachine and others added 4 commits June 23, 2021 10:44

Merge branch 'master' into es-kuery/new-interface

308ec01

rename

03ca054

Merge branch 'master' of github.com:elastic/kibana into es-kuery/new-…

903b8f4

…interface

Merge branch 'es-kuery/new-interface' of github.com:lizozom/kibana in…

68a7d27

…to es-kuery/new-interface

lizozom merged commit 77fe1c1 into elastic:master Jun 23, 2021

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jun 25, 2021

lizozom mentioned this pull request Jun 28, 2021

[7.x] [Query] Use a minimal index pattern interface for es query (#102364) #103453

Merged

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jun 28, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Query] Use a minimal index pattern interface for es query #102364

[Query] Use a minimal index pattern interface for es query #102364

lizozom commented Jun 16, 2021

lizozom Jun 16, 2021

mattkime Jun 16, 2021

lukeelmers left a comment

lukeelmers Jun 16, 2021

lizozom Jun 23, 2021

elasticmachine commented Jun 23, 2021

mattkime left a comment

lizozom commented Jun 23, 2021

kibanamachine commented Jun 23, 2021

API count

kibanamachine commented Jun 23, 2021

kibanamachine commented Jun 25, 2021


		import { IFieldType } from '../../index_patterns';

		export interface MinimalIndexPattern {

[Query] Use a minimal index pattern interface for es query #102364

[Query] Use a minimal index pattern interface for es query #102364

Conversation

lizozom commented Jun 16, 2021

Summary

Checklist

Risk Matrix

For maintainers

lizozom Jun 16, 2021

Choose a reason for hiding this comment

mattkime Jun 16, 2021

Choose a reason for hiding this comment

lukeelmers left a comment

Choose a reason for hiding this comment

lukeelmers Jun 16, 2021

Choose a reason for hiding this comment

lizozom Jun 23, 2021

Choose a reason for hiding this comment

elasticmachine commented Jun 23, 2021

mattkime left a comment

Choose a reason for hiding this comment

lizozom commented Jun 23, 2021

kibanamachine commented Jun 23, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

API count

History

kibanamachine commented Jun 23, 2021

💔 Backport failed

kibanamachine commented Jun 25, 2021