-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Logs UI][Metrics UI] Replace usage of deprecated IndexPattern types #114448
[Logs UI][Metrics UI] Replace usage of deprecated IndexPattern types #114448
Conversation
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.
At first glance this looks great. 👍 I left just two questions below.
@@ -82,7 +83,7 @@ class WithKueryAutocompletionComponent extends React.Component< | |||
query: expression, | |||
selectionStart: cursorPosition, | |||
selectionEnd: cursorPosition, | |||
indexPatterns: [indexPattern], | |||
indexPatterns: [indexPattern as any], // How to fix this without changing the dependency? |
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.
Instead of casting to any
, how about we use a // @ts-expect-error
? That could increase the likelihood that the person updating the getQuerySuggestions
signature will be prompted by CI to remove the workaround.
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.
Much better, thanks!
@@ -5,7 +5,8 @@ | |||
* 2.0. | |||
*/ | |||
|
|||
import { IndexPattern, KBN_FIELD_TYPES } from '../../../../../../../src/plugins/data/public'; | |||
import { DataView } from '../../../../../../../src/plugins/data/common'; |
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.
Why not keep importing from public
? I'm not sure cross-plugin imports from common
into public
are officially supported. In this case it seems easy to avoid them.
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.
Right, that was sloppy of me!
It's because the data
plugin doesn't export DataView
from public, but in fact that type lives within the data_view
plugin and there it is exported from public.
The data
plugin only re-exports parts of the data_views
types (like IndexPattern
).
I'll fix that :)
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.
Looking at the other files, they seem to pull mostly form /common, is that something I should look to change as well?
Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui) |
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]History
To update your PR or re-run it, just comment with: |
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.
It looks good and builds correctly, well done.
…lastic#114448) * [Logs UI][Metrics UI] Replace usage of deprecated IndexPattern types (elastic#107887)
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
…114448) (#114590) * [Logs UI][Metrics UI] Replace usage of deprecated IndexPattern types (#107887) Co-authored-by: Milton Hultgren <[email protected]>
Summary
Closes #107887
Replaces usages of the following deprecated types in the Infra plugin code:
IIndexPattern
IndexPattern
IndexPatternField
IndexPatternsContract
Concerns:
Not sure what to set 'type' to, it's set to undefined unless the index is for a rollup
The data plugin still uses the old IndexPattern types so for now I cast to any but that feels bad
Follow up:
We'll need to rename our functions/variables to match the new concepts as everything is still called "index pattern" in our code