Index pattern serialize and de-serialize by mattkime · Pull Request #68844 · elastic/kibana

mattkime · 2020-06-11T01:59:09Z

Summary

IndexPattern.toSpec() and IndexPatternService.specToIndexPattern(indexPatternSpec)

part of #68003

Checklist

Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

ppisljar

minor comments but really excited to see this !

src/plugins/data/common/index_patterns/index_patterns/index_patterns.ts

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

ppisljar · 2020-06-11T15:17:16Z

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

+
+export interface IndexPatternSpec {
+  _id: string;
+  _type: string;


whats a type ? are this saved object properties ?

yes, its a saved object property.

ppisljar · 2020-06-11T15:17:28Z

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

+  _id: string;
+  _type: string;
+  _source: {
+    title: string;


why are this under _source ?

feels like we might just want whats under _source ? could we restore index pattern withoiut the other stuff ?

@ppisljar I agree that we could do that and it would likely be sufficient for your needs. However, that would be a different IndexPattern - one that couldn't be saved. I'd prefer to have just one kind of index pattern. We could consider having different types after some of the planned refactoring.

what do you mean it couldn't be saved ? if

export interface IndexPatternSpec { id: string; title: string; timeFieldName?: string; fields: FieldSpec[]; version: string; }

and then specToIndexPattern takes this in and gives you back indexPattern instance that can be saved.

lets try to make this IndexPatternSpec a bit easier to handle and more futureproof if possible. the information on how saved object looks like does not need to be exposed to the consumer (IndexPatternSpec can have a different shape as long as it contains sufficient information to reconstruct index pattern out of it)

version can also be removed as that's a saved object thing

I guess I didn't understand what you were asking for, happy to implement IndexPatternSpec as explained above.

That said, I think we need to keep version as its used when updating an index pattern saved object.

we should always assume the latest version when creating index pattern from spec.

version is used to ensure there aren't write collisions.

This reverts commit f4aadbe.

This reverts commit 8239563.

elasticmachine · 2020-06-12T04:43:30Z

Pinging @elastic/kibana-app-arch (Team:AppArch)

mattkime · 2020-06-15T15:21:54Z

@elasticmachine merge upstream

mattkime · 2020-06-23T18:20:21Z

I'm also not clear on why we have version since we'd always be creating the latest version from the spec. @mattkime you had mentioned this was to avoid write conflicts; could you provide more background on the use case there?

Here's an example version value - WzE3NTgsMV0= - the specific value, as best I can tell, is random, or maybe its a hash of the saved object content. It changes each time the saved object is updated. If you try to save but version doesn't match it will be rejected - someone else already updated it.

Why does this pertain to IndexPatterns? It has logic for reattempting saving when there are conflicting save attempts.

perhaps I should change the value in IndexPatterns to be savedObjectVersion

mattkime · 2020-06-23T18:35:54Z

Overall naming consistency nit

FieldFormats uses .toJSON as best I can tell. To me, serialized/deserialize means to and from string. Same for .toJSON although it returns a spec. That said, I'm happy to go along with how its done elsewhere.

lukeelmers · 2020-06-23T19:21:46Z

To me, serialized/deserialize means to and from string

Yeah I know @ppisljar and I have discussed this before too. Since technically we are returning serializable state but not serialized state. I've seen usage of toJSON as well, which is handy in the case where you are actually serializing it (via JSON.stringify).

This is something I don't feel strongly about either, just falls into the same category for me of "stuff we should try to make consistent"

Perhaps toJSON and fromJSON instead of serialize and deserialize? @ppisljar do you have any thoughts on this?

mattkime · 2020-06-23T19:25:40Z

Perhaps toJSON and fromJSON instead of serialize and deserialize?

I literally can't use that with the Field class because its used for serializing to saved object which is a different format. Index Pattern uses it as well but it just returns the id.

…bject

mattkime · 2020-06-23T19:58:10Z

++ I realize the existing type for this is Record<string, any>, but it would be really nice if it could be tightened up based on the return value of toSpec.

I'm going to see if I can tighten up the types on toSpec without impacting other code.

ppisljar · 2020-06-24T08:34:29Z

i don't have preference about naming the functions, but i agree it would be nice if we would be consistent about it. Actually toSpec might be the best suggestion so far, i don't like serialize as it doesn't serialize, toJSON is nice as Luke mentions but can't be used everywhere.

mattkime · 2020-06-24T13:12:17Z

@elasticmachine merge upstream

kibanamachine · 2020-06-24T14:52:56Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 7d0151a

Build metrics

@kbn/optimizer bundle module count

id	value	diff	baseline
data	207	-1	208

History

💔 Build #55811 failed 392508c

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

ppisljar

code LGTM

lukeelmers

Changes LGTM! Using toSpec and fromSpec works for me - we can look into changing those in aggs & visualize for consistency.

kertal

KibanaApp Code LGTM, didn't test the test, I trust good ol' jenkins to do so

* master: (90 commits) [Encrypted Saved Objects] Adds support for migrations in ESO (elastic#69513) [SIEM] Replace WithSource with useWithSource hook (elastic#68722) [Endpoint]EMT-451: add ability to filter endpoint metadata based on presence of unenrolled events (elastic#69708) rename old siem kibana config to securitySolution (elastic#69874) Remove unused Resolver code (elastic#69914) [Observability] Fixing dynamic return type based on the appName (elastic#69894) [SECURITY SOLUTION][INGEST] Task/endpoint list tests (elastic#69419) Fixes special clicks and 3rd party icon sizes in nav (elastic#69767) [APM] Catch annotations index permission error and log warning (elastic#69881) [Endpoint][Ingest Manager] minor code cleanup (elastic#69844) [Logs UI] Logs ui context menu (elastic#69915) Index pattern serialize and de-serialize (elastic#68844) [QA] Unskip functional tests (elastic#69760) [SIEM][Detection Engine] - Update DE to work with new exceptions schema (elastic#69715) Fixes elastic#69639: Ignore url.url fields above 2048 characters (elastic#69863) PR: Provide limit warnings to user when API limits are reached. (elastic#69590) [Maps] Remove broken button (elastic#69853) Makes usage collection methods available on start (elastic#69836) [SIEM][CASE] Improve Jira's labelling (elastic#69892) [Logs UI] Access ML via the programmatic plugin API (elastic#68905) ...

* serialize and deserialize index patterns # Conflicts: # src/plugins/data/common/index_patterns/index_patterns/index_pattern.ts # src/plugins/data/common/index_patterns/index_patterns/index_patterns.ts # src/plugins/data/common/index_patterns/types.ts # src/plugins/data/public/index.ts

* Index pattern serialize and de-serialize (#68844)

* master: [Encrypted Saved Objects] Adds support for migrations in ESO (elastic#69513) [SIEM] Replace WithSource with useWithSource hook (elastic#68722) [Endpoint]EMT-451: add ability to filter endpoint metadata based on presence of unenrolled events (elastic#69708) rename old siem kibana config to securitySolution (elastic#69874) Remove unused Resolver code (elastic#69914) [Observability] Fixing dynamic return type based on the appName (elastic#69894) [SECURITY SOLUTION][INGEST] Task/endpoint list tests (elastic#69419) Fixes special clicks and 3rd party icon sizes in nav (elastic#69767) [APM] Catch annotations index permission error and log warning (elastic#69881) [Endpoint][Ingest Manager] minor code cleanup (elastic#69844) [Logs UI] Logs ui context menu (elastic#69915) Index pattern serialize and de-serialize (elastic#68844)

mattkime added 5 commits June 10, 2020 20:57

serialize and deserialize index patterns

a63107a

serialize and deserialize index patterns

75f6b12

Merge branch 'master' into index_pattern_serialize

3ad8c2f

just a comment

4eecca4

Merge branch 'master' into index_pattern_serialize

37a4b7f

ppisljar requested changes Jun 11, 2020

View reviewed changes

remove underscores in front of vars

ca88133

mattkime changed the title ~~Index pattern serialize~~ Index pattern serialize and de-serialize Jun 11, 2020

mattkime added 9 commits June 11, 2020 17:54

add tests

907e21b

complete tests

9f6e464

update docs

508625f

update docs

fec59e1

test cleanup

33a1c24

export IndexPatternSpec

f4aadbe

Revert "export IndexPatternSpec"

8239563

This reverts commit f4aadbe.

Revert "Revert "export IndexPatternSpec""

af2eb4d

This reverts commit 8239563.

fix bad import

3477987

mattkime marked this pull request as ready for review June 12, 2020 04:27

mattkime requested a review from a team as a code owner June 12, 2020 04:27

mattkime added the Team:AppArch label Jun 12, 2020

mattkime added Feature:Data Views Data Views code and UI - index patterns before 8.0 v8.0.0 v7.9.0 release_note:skip Skip the PR/issue when compiling release notes labels Jun 12, 2020

mattkime added 3 commits June 12, 2020 07:03

one less any

beb6c0b

update docs

c7c3ef3

remove unneeded change

390f833

Merge branch 'master' into index_pattern_serialize

71576fb

remove comment, better type, add Field test

8fa135d

mattkime added 2 commits June 23, 2020 14:32

better saved object types, remove comment, and format always has an o…

2517422

…bject

field format defaults to undefined

70c2c60

mattkime added 6 commits June 23, 2020 15:55

provide types for field toSpec

be57066

update docs

24d0516

accept toSpec format to make Field

badd5e8

update docs

6535713

use serializedFieldFormat instead of FieldFormatSpec

d2eedb8

type fixes

392508c

elastic deleted a comment from kibanamachine Jun 24, 2020

Merge branch 'master' into index_pattern_serialize

7d0151a

ppisljar approved these changes Jun 24, 2020

View reviewed changes

lukeelmers approved these changes Jun 24, 2020

View reviewed changes

kertal approved these changes Jun 25, 2020

View reviewed changes

mattkime merged commit b02e2d9 into elastic:master Jun 25, 2020

mattkime mentioned this pull request Jun 25, 2020

[7.x] Index pattern serialize and de-serialize (#68844) #69979

Merged

mattkime added a commit that referenced this pull request Jun 25, 2020

[7.x] Index pattern serialize and de-serialize (#68844) (#69979)

7548991

* Index pattern serialize and de-serialize (#68844)

mattkime mentioned this pull request Jul 13, 2020

[Meta] Index Patterns #67574

Closed

12 tasks

mattkime mentioned this pull request Sep 24, 2020

[Meta] Index Patterns Roadmap #78376

Closed

63 tasks

Conversation

mattkime commented Jun 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

For maintainers

Uh oh!

ppisljar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ppisljar Jun 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattkime Jun 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Jun 12, 2020

Uh oh!

mattkime commented Jun 15, 2020

Uh oh!

mattkime commented Jun 23, 2020

Uh oh!

mattkime commented Jun 23, 2020

Uh oh!

lukeelmers commented Jun 23, 2020

Uh oh!

mattkime commented Jun 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattkime commented Jun 23, 2020

Uh oh!

ppisljar commented Jun 24, 2020

Uh oh!

mattkime commented Jun 24, 2020

Uh oh!

kibanamachine commented Jun 24, 2020

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

History

Uh oh!

ppisljar left a comment

Choose a reason for hiding this comment

Uh oh!

lukeelmers left a comment

Choose a reason for hiding this comment

Uh oh!

kertal left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mattkime commented Jun 11, 2020 •

edited

Loading

ppisljar Jun 16, 2020 •

edited

Loading

mattkime Jun 16, 2020 •

edited

Loading

mattkime commented Jun 23, 2020 •

edited

Loading