-
Notifications
You must be signed in to change notification settings - Fork 870
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
[Multiple Datasource][Version Decoupling] Support Version Decoupling in Index Patterns Dashboards Plugin #7100
Changes from 2 commits
b17902a
fdf8a86
1305621
e649ca0
2b6db3b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
fix: | ||
- [MDS][Version Decoupling] Add support of Version Decoupling in Index Patterns Dashboards Plugin ([#7100](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7100)) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,10 +23,12 @@ | |
DataSourceRef, | ||
IndexPatternManagmentContext, | ||
} from 'src/plugins/index_pattern_management/public/types'; | ||
import semver from 'semver'; | ||
import { useOpenSearchDashboards } from '../../../../../../../../../plugins/opensearch_dashboards_react/public'; | ||
import { getDataSources } from '../../../../../../components/utils'; | ||
import { DataSourceTableItem, StepInfo } from '../../../../types'; | ||
import { LoadingState } from '../../../loading_state'; | ||
import * as pluginManifest from '../../../../../../../opensearch_dashboards.json'; | ||
|
||
interface HeaderProps { | ||
onDataSourceSelected: (id: string, type: string, title: string) => void; | ||
|
@@ -68,6 +70,12 @@ | |
.then((fetchedDataSources: DataSourceTableItem[]) => { | ||
setIsLoading(false); | ||
if (fetchedDataSources?.length) { | ||
fetchedDataSources = fetchedDataSources.filter((dataSource) => | ||
semver.satisfies( | ||
Check warning on line 74 in src/plugins/index_pattern_management/public/components/create_index_pattern_wizard/components/step_data_source/components/header/header.tsx Codecov / codecov/patchsrc/plugins/index_pattern_management/public/components/create_index_pattern_wizard/components/step_data_source/components/header/header.tsx#L73-L74
|
||
dataSource.datasourceversion, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what about for existing/old data sources that doesn't have this field? Does it break backwards compatibility? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for older data sources it won't work yest as they won't support version decoupling, MDS initially just list out all data sources created regardless so there would be a bunch of unexpected behaviors, version decoupling is aiming to improve that user experiences by only allowing consumption against compatible data sources. besides there is a |
||
pluginManifest.supportedOSDataSourceVersions | ||
) | ||
); | ||
setDataSources(fetchedDataSources); | ||
} | ||
}) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -92,4 +92,5 @@ export interface DataSourceTableItem { | |
sort: string; | ||
checked?: 'on' | 'off'; | ||
label: string; | ||
datasourceversion: string; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it expected that each plugin needs to have this change in order to onboard version decoupling? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, datasource would be the minimal fields to check in order to decide the version compatibility |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -90,7 +90,7 @@ | |
savedObjectsClient | ||
.find<DataSourceAttributes>({ | ||
type: 'data-source', | ||
fields: ['title', 'type'], | ||
fields: ['title', 'type', 'dataSourceVersion'], | ||
perPage: 10000, | ||
}) | ||
.then((response) => | ||
|
@@ -99,13 +99,15 @@ | |
const id = dataSource.id; | ||
const type = dataSource.type; | ||
const title = dataSource.get('title'); | ||
const datasourceversion = dataSource.get('dataSourceVersion'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as above. |
||
|
||
return { | ||
id, | ||
title, | ||
type, | ||
label: title, | ||
sort: `${title}`, | ||
datasourceversion, | ||
}; | ||
}) | ||
.sort((a, b) => a.sort.localeCompare(b.sort)) | ||
|
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.
nit: can we use absolute path here instead of relative path with directory traversal?
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.
yes, this one just to keep the save relative style as existing imports lol