-
Notifications
You must be signed in to change notification settings - Fork 0
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
Modify to use loader to get index pattern & Add dataSourceRef #20
Conversation
@@ -86,7 +92,8 @@ export class IndexPattern implements IIndexPattern { | |||
// savedObject version | |||
public version: string | undefined; | |||
public sourceFilters?: SourceFilter[]; | |||
public dataSourcesJSON: string | undefined; | |||
public dataSourcesJSON?: string; // todo: cleanup, only keep one |
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 share more about what's blocking you to clean up this field? I mean we don't need to be rush, I can wait for you to complete the clean up and then merge my PR.
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.
The dataSourcesJSON is currently used by the UI side as well, so to clean it up, extra work is involved, however, for your side, just rely on dataSourceRef
will do the trick. -- I will merge this PR, so we move fast to testing the read from dataSourceRef on visualization side. Just wanna callout that the cleanup is transparent to your side :)
} | ||
|
||
getDataSourceRef = (dataSourcesJSON?: string) => { |
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.
If it's just extracting some info from another field dataSourcesJSON
. It can be done anywhere, I can do it from my side too. I wonder is it possible to get rid of the dataSourcesJSON
in the data model itself.
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.
I think you are following similar pattern to add xxxJSON
to any saved object mapping and interfaces(E.g. see below dashboard.ts
). But I didn't see this JSON field being added to the actual index-pattern mapping defined in index_patterns.ts
? I am still trying to understand the codebase, so correct me if I am wrong.
OpenSearch-Dashboards/src/plugins/dashboard/server/saved_objects/dashboard.ts
Lines 57 to 65 in e41efcc
mappings: { | |
properties: { | |
description: { type: 'text' }, | |
hits: { type: 'integer', index: false, doc_values: false }, | |
kibanaSavedObjectMeta: { | |
properties: { searchSourceJSON: { type: 'text', index: false } }, | |
}, | |
optionsJSON: { type: 'text', index: false }, | |
panelsJSON: { type: 'text', index: false }, |
OpenSearch-Dashboards/src/plugins/data/server/saved_objects/index_patterns.ts
Lines 59 to 65 in e41efcc
mappings: { | |
dynamic: false, | |
properties: { | |
title: { type: 'text' }, | |
type: { type: 'keyword' }, | |
}, | |
}, |
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.
This is a good question, index pattern indeed have lots fields not added to map, e,g, fields, timeFieldName etc. My guess is it's due to they are optional fields.
Description
Test
Tested to fetch a index pattern in data source inside visualization:
Issues Resolved
TBA