[Discover] Filter out OpenSearch Serverless Collections #8758

sejli · 2024-10-30T19:06:15Z

Description

Filters out OpenSearch Serverless Collections after fetching data sources

Issues Resolved

Screenshot

Testing the changes

Changelog

skip

Check List

All tests pass
- yarn test:jest
- yarn test:jest_integration
New functionality includes testing.
New functionality has been documented.
Update CHANGELOG.md
Commits are signed per the DCO using --signoff

Signed-off-by: Sean Li <[email protected]>

ashwin-pc

Lgtm

codecov · 2024-10-30T19:22:18Z

Codecov Report

Attention: Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 60.75%. Comparing base (48bfe2c) to head (58efea4).
Report is 9 commits behind head on main.

Files with missing lines	Patch %	Lines
...ery/query_string/dataset_service/lib/index_type.ts	0.00%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #8758   +/-   ##
=======================================
  Coverage   60.75%   60.75%           
=======================================
  Files        3798     3798           
  Lines       90684    90690    +6     
  Branches    14272    14277    +5     
=======================================
+ Hits        55091    55099    +8     
+ Misses      32094    32091    -3     
- Partials     3499     3500    +1

Flag	Coverage Δ
Linux_1	`29.05% <0.00%> (-0.01%)`	⬇️
Linux_2	`56.39% <ø> (ø)`
Linux_3	`37.60% <0.00%> (+<0.01%)`	⬆️
Linux_4	`29.82% <50.00%> (+<0.01%)`	⬆️
Windows_1	`29.07% <0.00%> (-0.01%)`	⬇️
Windows_2	`56.34% <ø> (ø)`
Windows_3	`37.61% <0.00%> (+<0.01%)`	⬆️
Windows_4	`29.82% <50.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Sean Li <[email protected]>

kavilla · 2024-10-30T20:31:11Z

src/plugins/data/public/query/query_string/dataset_service/lib/index_type.ts

-    }))
+    response.savedObjects
+      .filter(
+        (savedObject) => savedObject.attributes.dataSourceEngineType !== 'OpenSearch Serverless'


if we got the okay to filter out serverless then this looks good to me!

definitely not a blocker for this PR or more of a question: do we think it would be worth it for the dataset type config to have a supportedEngineType (following the naming pattern set by @manasvinibs) a while back?

a value we could respect is ALL and all the data source engine types work. that way if the engine type and i think it actually should follow the complete paradigm set by mana. where it actually just be supportedEngines which is a record of semver engines.

this will help future proof us like

supportedEngines: { 'OpenSearch': "^1.0.0", }

for scenarios like maybe a specific feature on a cluster.

cc: @virajsanghvi @AMoo-Miki any opinions on this one

A. If AWS wants to filter something out, they are more than welcome to patch it in their fork. OpenSearch logic should not contain opinions for AWS. OSD is not the one who defined dataSourceEngineType: 'OpenSearch Serverless' so it shouldn't be the one who filters it out.
B. @kavilla, while i like the flexibility your proposal provides, without many users asking for this flexibility, I cannot convince myself that it is worth complicating the code.

yeah that's true. something that we sometimes run into is like the query assist be available on only certain clusters and version.

but yeah that would be too complicated and i think we would should solve that more universally with capability APIs

Agree this should ideally be capabilities based

can this use the enum instead of string

OpenSearch-Dashboards/src/plugins/data_source/common/data_sources/types.ts

Line 60 in a70a7b8

OpenSearchServerless = 'OpenSearch Serverless',

OpenSearch logic should not contain opinions for AWS

If OSD allows users to add OpenSearch serverless as datasources to it, I think this is not an opinion for AWS, it's a limitation that this combination does not work in OSD?

OSD is not the one who defined dataSourceEngineType: 'OpenSearch Serverless'

q: who is defining the dataSourceEngineType: 'OpenSearch Serverless'

Should be the data_source plugin when a data source is created.

const handleSubmit = async (attributes: DataSourceAttributes) => { setIsLoading(true); try { // Fetch data source metadata from added OS/ES domain/cluster const metadata = await fetchDataSourceMetaData(http, attributes); attributes.dataSourceVersion = metadata.dataSourceVersion; attributes.dataSourceEngineType = metadata.dataSourceEngineType; attributes.installedPlugins = metadata.installedPlugins;

If OSD allows users to add OpenSearch serverless as datasources to it, I think this is not an opinion for AWS, it's a limitation that this combination does not work in OSD?

Thanks for finding that. It should have never happened.

who is defining the dataSourceEngineType: 'OpenSearch Serverless'

Found it in the data_source:

OpenSearch-Dashboards/src/plugins/data_source/server/routes/data_source_connection_validator.ts

Lines 41 to 71 in 190dab0

async fetchDataSourceInfo() {

const dataSourceInfo: DataSourceInfo = {

dataSourceVersion: '',

dataSourceEngineType: DataSourceEngineType.NA,

};

try {

// OpenSearch Serverless does not have version concept

if (

this.dataSourceAttr.auth?.credentials?.service === SigV4ServiceName.OpenSearchServerless

) {

dataSourceInfo.dataSourceEngineType = DataSourceEngineType.OpenSearchServerless;

return dataSourceInfo;

}

await this.callDataCluster

.info()

.then((response) => response.body)

.then((body) => {

dataSourceInfo.dataSourceVersion = body.version.number;

if (

body.version.distribution !== null &&

body.version.distribution !== undefined &&

body.version.distribution === 'opensearch'

) {

dataSourceInfo.dataSourceEngineType = DataSourceEngineType.OpenSearch;

} else {

dataSourceInfo.dataSourceEngineType = DataSourceEngineType.Elasticsearch;

}

});

Apparently all of these was added to OSD :/

src/plugins/query_enhancements/public/datasets/s3_type.ts

Signed-off-by: Sean Li <[email protected]>

ananzh

We could a follow up on @kavilla's question

virajsanghvi

How did we test these changes?

virajsanghvi · 2024-10-30T21:05:56Z

src/plugins/query_enhancements/public/datasets/s3_type.ts

-    }))
+    resp.savedObjects
+      .filter(
+        (savedObject) => savedObject.attributes.dataSourceEngineType !== 'OpenSearch Serverless'


Is attributes guaranteed to be defined?

Attributes is guaranteed to be defined, but not sure about dataSourceEngineType. This should be some of the data_source plugin work using the saved objects client. I'm not super familiar, but it looks like it should also be defined for data sources here.

.then((response) => { const objects = response?.savedObjects; if (objects) { return objects.map((source) => { const id = source.id; // Data connection doesn't have title for now, would use connectionId instead to display. const title = source.get('title') ?? source.get('connectionId') ?? ''; const workspaces = source.workspaces ?? []; const auth = source.get('auth'); const description = source.get('description'); const dataSourceEngineType = source.get('dataSourceEngineType'); const type = source.type;

kavilla · 2024-10-30T22:53:37Z

src/plugins/data/public/query/query_string/dataset_service/lib/index_type.ts

-      title: savedObject.attributes.title,
-      type: 'DATA_SOURCE',
-    }))
+    response.savedObjects


i think this file has tests as well we can add which should be copy pasta from the other one

but it can be another PR

kavilla · 2024-10-30T22:54:08Z

i see test failures on the ci but i feel like github actions have been having some issue yesterday and today

Signed-off-by: Sean Li <[email protected]>

* Filtering out serverless collections Signed-off-by: Sean Li <[email protected]> * rerunning linter Signed-off-by: Sean Li <[email protected]> * adding test for s3 type Signed-off-by: Sean Li <[email protected]> * updating filter Signed-off-by: Sean Li <[email protected]> * string check Signed-off-by: Sean Li <[email protected]> * update test Signed-off-by: Sean Li <[email protected]> --------- Signed-off-by: Sean Li <[email protected]> (cherry picked from commit 04dff9b) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* Filtering out serverless collections * rerunning linter * adding test for s3 type * updating filter * string check * update test --------- (cherry picked from commit 04dff9b) Signed-off-by: Sean Li <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…roject#8758) * Filtering out serverless collections Signed-off-by: Sean Li <[email protected]> * rerunning linter Signed-off-by: Sean Li <[email protected]> * adding test for s3 type Signed-off-by: Sean Li <[email protected]> * updating filter Signed-off-by: Sean Li <[email protected]> * string check Signed-off-by: Sean Li <[email protected]> * update test Signed-off-by: Sean Li <[email protected]> --------- Signed-off-by: Sean Li <[email protected]>

Filtering out serverless collections

4623841

Signed-off-by: Sean Li <[email protected]>

sejli added backport 2.x discover for discover reinvent backport 2.18 labels Oct 30, 2024

github-actions bot added seasoned-contributor Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry labels Oct 30, 2024

ashwin-pc previously approved these changes Oct 30, 2024

View reviewed changes

rerunning linter

ff7cad6

Signed-off-by: Sean Li <[email protected]>

sejli dismissed ashwin-pc’s stale review via ff7cad6 October 30, 2024 19:42

ananzh added the v2.18.0 label Oct 30, 2024

kavilla reviewed Oct 30, 2024

View reviewed changes

src/plugins/query_enhancements/public/datasets/s3_type.ts Outdated Show resolved Hide resolved

adding test for s3 type

ecfba30

Signed-off-by: Sean Li <[email protected]>

ananzh previously approved these changes Oct 30, 2024

View reviewed changes

virajsanghvi reviewed Oct 30, 2024

View reviewed changes

kavilla previously approved these changes Oct 30, 2024

View reviewed changes

kavilla reviewed Oct 30, 2024

View reviewed changes

ananzh self-requested a review October 30, 2024 23:36

updating filter

dc6cb28

Signed-off-by: Sean Li <[email protected]>

sejli dismissed stale reviews from kavilla and ananzh via dc6cb28 October 30, 2024 23:39

sejli added 2 commits October 30, 2024 16:44

string check

848bcdf

Signed-off-by: Sean Li <[email protected]>

update test

58efea4

Signed-off-by: Sean Li <[email protected]>

kavilla approved these changes Oct 30, 2024

View reviewed changes

ashwin-pc approved these changes Oct 31, 2024

View reviewed changes

ananzh merged commit 04dff9b into opensearch-project:main Oct 31, 2024
67 of 68 checks passed

opensearch-trigger-bot bot mentioned this pull request Oct 31, 2024

[Backport 2.x] [Discover] Filter out OpenSearch Serverless Collections #8773

Merged

opensearch-trigger-bot bot mentioned this pull request Oct 31, 2024

[Backport 2.18] [Discover] Filter out OpenSearch Serverless Collections #8774

Closed

kavilla mentioned this pull request Nov 1, 2024

[Discover] Update Version Filter for Data Sources #8785

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Discover] Filter out OpenSearch Serverless Collections #8758

[Discover] Filter out OpenSearch Serverless Collections #8758

sejli commented Oct 30, 2024

ashwin-pc left a comment

codecov bot commented Oct 30, 2024 •

edited

Loading

kavilla Oct 30, 2024

AMoo-Miki Oct 30, 2024 •

edited

Loading

kavilla Oct 30, 2024

virajsanghvi Oct 30, 2024

joshuali925 Oct 30, 2024

sejli Oct 31, 2024

AMoo-Miki Oct 31, 2024

ananzh left a comment

virajsanghvi left a comment

virajsanghvi Oct 30, 2024

sejli Oct 31, 2024

kavilla Oct 30, 2024

kavilla Oct 30, 2024

kavilla commented Oct 30, 2024

	async fetchDataSourceInfo() {
	const dataSourceInfo: DataSourceInfo = {
	dataSourceVersion: '',
	dataSourceEngineType: DataSourceEngineType.NA,
	};

	try {
	// OpenSearch Serverless does not have version concept
	if (
	this.dataSourceAttr.auth?.credentials?.service === SigV4ServiceName.OpenSearchServerless
	) {
	dataSourceInfo.dataSourceEngineType = DataSourceEngineType.OpenSearchServerless;
	return dataSourceInfo;
	}

	await this.callDataCluster
	.info()
	.then((response) => response.body)
	.then((body) => {
	dataSourceInfo.dataSourceVersion = body.version.number;

	if (
	body.version.distribution !== null &&
	body.version.distribution !== undefined &&
	body.version.distribution === 'opensearch'
	) {
	dataSourceInfo.dataSourceEngineType = DataSourceEngineType.OpenSearch;
	} else {
	dataSourceInfo.dataSourceEngineType = DataSourceEngineType.Elasticsearch;
	}
	});

[Discover] Filter out OpenSearch Serverless Collections #8758

[Discover] Filter out OpenSearch Serverless Collections #8758

Conversation

sejli commented Oct 30, 2024

Description

Issues Resolved

Screenshot

Testing the changes

Changelog

Check List

ashwin-pc left a comment

Choose a reason for hiding this comment

codecov bot commented Oct 30, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

AMoo-Miki Oct 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ananzh left a comment

Choose a reason for hiding this comment

virajsanghvi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kavilla commented Oct 30, 2024

codecov bot commented Oct 30, 2024 •

edited

Loading

AMoo-Miki Oct 30, 2024 •

edited

Loading