[Canvas] Use dataviews api in canvas #130320
Conversation
crob611
added
review
Team:Presentation
|
Pinging @elastic/kibana-presentation (Team:Presentation) |
crob611
changed the title
crob611
added
the
impact:low
crob611
force-pushed
the
canvas-data-views
branch
from
894d614 to
26bc6b9
Compare
💚 Build SucceededMetrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
| loading: boolean; | ||
| value: string; | ||
| indices: string[]; | ||
| onChange: (index: string) => void; | ||
| dataViews: string[]; |
There was a problem hiding this comment.
How are dataViews represented by strings? Just making sure will work in all circumstances.
| getDataViews: async () => { | ||
| try { | ||
| const dataViews = await startPlugins.data.dataViews.getIdsWithTitle(); | ||
| return dataViews.map((view) => view.title); |
There was a problem hiding this comment.
This code appears to assume that titles are unique. Only ids should be considered unique.
|
|
||
| return []; | ||
| }, | ||
| getFields: async (dataViewTitle: string) => { |
There was a problem hiding this comment.
Is this used exclusively when an existing data view doesn't exist? Why are all fields starting with _ being filtered out? It is valid to have an indexed field that starts with _.
There was a problem hiding this comment.
This is used when we have a title, and need to get the fields. The _ fields are filtered out as an attempt to remove meta fields it appears. Is there something formalized with the DataView api's to filter out meta fields? Is the create the correct route to go when we have only the title and need to fields?
There was a problem hiding this comment.
Is the user thinking in terms of data views or index pattern strings? If they're using data views then you should get the field list from the data view directly. Lets establish this and then discuss handling meta fields.
| .map((field) => field.name); | ||
| }, | ||
| getDefaultDataView: async () => { | ||
| const dataView = await startPlugins.data.dataViews.getDefaultDataView(); |
There was a problem hiding this comment.
title should not be assumed to be unique.
|
@mattkime Great points you bring up. Let me address here, and then maybe we sync up to come with a better plan moving forward. I hope that this PR simply ports the existing code to use the new data views api, but maybe we can fix some of the issues that this area has had from the beginning. The And selecting one will update the expression with that DataView's title That string value of "index" is then used as an index argument on a query to a Maybe this is something that doesn't need to use DataViews in that case, since we are using it as a straight up index pattern instead? Previously we were querying for Anyways, like I said, interested to hear your thoughts and we can sync up if we want to discuss further. |
|
@crob611 I think that makes sense since the title is being consumed in a particular way. I'm a little confused as to why |
|
Closing in favor of #139610. |
Part of #91715
This PR is pretty straightforward. Creates a DataViews service to replace the existing
es_servicesfile and updates all of the locations that use those services.