Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[discover] Clean up enhanced search API #8226

Merged
merged 2 commits into from
Sep 20, 2024
Merged

[discover] Clean up enhanced search API #8226

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
989990f
Clean up enhanced search API
kavilla Sep 18, 2024
d2992b2
Changeset file for PR #8226 created/updated
opensearch-changeset-bot[bot] Sep 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelogs/fragments/8226.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
fix:
- Remove unused enhancements APIs, clean up, and error handling ([#8226](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8226))
2 changes: 1 addition & 1 deletion src/plugins/data/common/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export const DEFAULT_DATA = {
} as DataStructure,
LOCAL_DATASOURCE: {
id: '',
title: 'Local Cluster',
title: 'Default Cluster',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont think this is correct. This should only show if there is a local cluster. The default cluster will be returned as a part of the datasources API

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah that makes sense i was worried making too big of a change in the case of no local cluster.

should we create an issue for this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the expected behaviour is that this option only shows up when you have a local cluster. If MDS is off the index list should be shown directly.

type: 'DATA_SOURCE',
},
},
Expand Down
5 changes: 1 addition & 4 deletions src/plugins/data/common/search/search_source/fetch/get_search_params.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,19 +53,16 @@
searchRequest: SearchRequest,
dependencies: {
getConfig: GetConfigFn;
getDataFrame: GetDataFrameFn;
}
): ISearchRequestParams {
const { getConfig, getDataFrame } = dependencies;
const { getConfig } = dependencies;

Check warning on line 58 in src/plugins/data/common/search/search_source/fetch/get_search_params.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/common/search/search_source/fetch/get_search_params.ts#L58 

Added line #L58 was not covered by tests
const searchParams = getSearchParams(getConfig);
const dataFrame = getDataFrame();
const indexTitle = searchRequest.index.title || searchRequest.index;

return {
index: indexTitle,
body: {
...searchRequest.body,
...(dataFrame && dataFrame?.name === indexTitle ? { df: dataFrame } : {}),
},
...searchParams,
};
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/data/public/query/query_string/dataset_service/lib/index_type.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export const indexTypeConfig: DatasetTypeConfig = {
const dataSources = await fetchDataSources(services.savedObjects.client);
return {
...dataStructure,
columnHeader: 'Cluster',
columnHeader: 'Clusters',
hasNext: true,
children: dataSources,
};
Expand Down
5 changes: 2 additions & 3 deletions src/plugins/query_enhancements/common/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,8 @@ export const API = {
GENERATE: `${BASE_API}/assist/generate`,
},
DATA_SOURCE: {
EXTERNAL: `${BASE_API}/datasource/external`,
ASYNC_JOBS: `${BASE_API}/datasource/jobs`,
CONNECTIONS: `${BASE_API}/datasource/connections`,
ASYNC_JOBS: `${BASE_API}/jobs`,
CONNECTIONS: `${BASE_API}/connections`,
},
};

Expand Down
29 changes: 22 additions & 7 deletions src/plugins/query_enhancements/public/datasets/s3_type.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/

import { HttpSetup, SavedObjectsClientContract } from 'opensearch-dashboards/public';
import { trimEnd } from 'lodash';
import {
DATA_STRUCTURE_META_TYPES,
DEFAULT_DATA,
Expand All @@ -14,7 +15,7 @@
DatasetField,
} from '../../../data/common';
import { DatasetTypeConfig, IDataPluginServices } from '../../../data/public';
import { DATASET, handleQueryStatus } from '../../common';
import { API, DATASET, handleQueryStatus } from '../../common';
import S3_ICON from '../assets/s3_mark.svg';

export const s3TypeConfig: DatasetTypeConfig = {
Expand Down Expand Up @@ -115,7 +116,9 @@
try {
const response = await handleQueryStatus({
fetchStatus: () =>
http.fetch('../../api/enhancements/datasource/jobs', {
http.fetch({

Check warning on line 119 in src/plugins/query_enhancements/public/datasets/s3_type.ts

View check run for this annotation

Codecov / codecov/patch

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

Added line #L119 was not covered by tests
method: 'GET',
path: trimEnd(`${API.DATA_SOURCE.ASYNC_JOBS}`),
query: {
id: dataSource?.id,
queryId: meta.queryId,
Expand Down Expand Up @@ -171,9 +174,13 @@
http: HttpSetup,
dataSource: DataStructure
): Promise<DataStructure[]> => {
const query = (dataSource.meta as DataStructureCustomMeta).query;
const response = await http.fetch(`../../api/enhancements/datasource/external`, {
query,
const abortController = new AbortController();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do we do with this abort controller?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to verify if using it correctly yet. but in the case of the user selecting a connection accidentally and then selecting another connection immediately aborting the previous call

const query =
dataSource.id !== '' ? (dataSource.meta as DataStructureCustomMeta).query : undefined;
const response = await http.fetch({
method: 'GET',
path: trimEnd(`${API.DATA_SOURCE.CONNECTIONS}/${query?.id || ''}`),
signal: abortController.signal,
});

return response
Expand All @@ -190,10 +197,13 @@
};

const fetchDatabases = async (http: HttpSetup, path: DataStructure[]): Promise<DataStructure[]> => {
const abortController = new AbortController();

Check warning on line 200 in src/plugins/query_enhancements/public/datasets/s3_type.ts

View check run for this annotation

Codecov / codecov/patch

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

Added line #L200 was not covered by tests
const dataSource = path.find((ds) => ds.type === 'DATA_SOURCE');
const connection = path[path.length - 1];
const meta = connection.meta as DataStructureCustomMeta;
const response = await http.post(`../../api/enhancements/datasource/jobs`, {
const response = await http.fetch({

Check warning on line 204 in src/plugins/query_enhancements/public/datasets/s3_type.ts

View check run for this annotation

Codecov / codecov/patch

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

Added line #L204 was not covered by tests
method: 'POST',
path: trimEnd(`${API.DATA_SOURCE.ASYNC_JOBS}`),
body: JSON.stringify({
lang: 'sql',
query: `SHOW DATABASES in ${connection.title}`,
Expand All @@ -203,6 +213,7 @@
query: {
id: dataSource?.id,
},
signal: abortController.signal,
});

connection.meta = setMeta(connection, response);
Expand All @@ -211,11 +222,14 @@
};

const fetchTables = async (http: HttpSetup, path: DataStructure[]): Promise<DataStructure[]> => {
const abortController = new AbortController();

Check warning on line 225 in src/plugins/query_enhancements/public/datasets/s3_type.ts

View check run for this annotation

Codecov / codecov/patch

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

Added line #L225 was not covered by tests
const dataSource = path.find((ds) => ds.type === 'DATA_SOURCE');
const connection = path.find((ds) => ds.type === 'CONNECTION');
const sessionId = (connection?.meta as DataStructureCustomMeta).sessionId;
const database = path[path.length - 1];
const response = await http.post(`../../api/enhancements/datasource/jobs`, {
const response = await http.fetch({

Check warning on line 230 in src/plugins/query_enhancements/public/datasets/s3_type.ts

View check run for this annotation

Codecov / codecov/patch

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

Added line #L230 was not covered by tests
method: 'POST',
path: trimEnd(`${API.DATA_SOURCE.ASYNC_JOBS}`),
body: JSON.stringify({
lang: 'sql',
query: `SHOW TABLES in ${database.title}`,
Expand All @@ -225,6 +239,7 @@
query: {
id: dataSource?.id,
},
signal: abortController.signal,
});

database.meta = setMeta(database, response);
Expand Down
84 changes: 0 additions & 84 deletions src/plugins/query_enhancements/public/services/connections_service.ts
View file
Open in desktop

This file was deleted.

2 changes: 0 additions & 2 deletions src/plugins/query_enhancements/public/services/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,3 @@ import { createGetterSetter } from '../../../opensearch_dashboards_utils/common'

export const [getStorage, setStorage] = createGetterSetter<DataStorage>('storage');
export const [getData, setData] = createGetterSetter<DataPublicPluginStart>('data');

export { ConnectionsService } from './connections_service';
5 changes: 0 additions & 5 deletions src/plugins/query_enhancements/public/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,3 @@ export interface Connection {
auth?: any;
};
}

export interface ConnectionsServiceDeps {
http: CoreSetup['http'];
startServices: Promise<[CoreStart, QueryEnhancementsPluginStartDependencies, unknown]>;
}
62 changes: 13 additions & 49 deletions src/plugins/query_enhancements/server/routes/data_source_connection/routes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,62 +13,26 @@ export function registerDataSourceConnectionsRoutes(
) {
router.get(
{
path: API.DATA_SOURCE.CONNECTIONS,
validate: {
params: schema.object({}, { unknowns: 'allow' }),
},
},
async (context, request, response) => {
const fields = ['id', 'title', 'auth.type'];
const resp = await context.core.savedObjects.client.find({
type: 'data-source',
fields,
perPage: 10000,
});

return response.ok({ body: { savedObjects: resp.saved_objects } });
}
);

router.get(
{
path: `${API.DATA_SOURCE.CONNECTIONS}/{dataSourceId}`,
path: `${API.DATA_SOURCE.CONNECTIONS}/{id?}`,
validate: {
params: schema.object({
dataSourceId: schema.string(),
}),
},
},
async (context, request, response) => {
const resp = await context.core.savedObjects.client.get(
'data-source',
request.params.dataSourceId
);
return response.ok({ body: resp });
}
);

router.get(
{
path: `${API.DATA_SOURCE.EXTERNAL}`,
validate: {
query: schema.object({
id: schema.string(),
name: schema.nullable(schema.string()),
id: schema.maybe(schema.string()),
}),
},
},
async (context, request, response) => {
const client = request.query.id
? context.dataSource.opensearch.legacy.getClient(request.query.id).callAPI
const client = request.params.id
? context.dataSource.opensearch.legacy.getClient(request.params.id).callAPI
: defaultClient.asScoped(request).callAsCurrentUser;

const resp = request.query.name
? await client('enhancements.getDataConnectionById', {
dataconnection: request.query.name,
})
: await client('enhancements.getDataConnections');
return response.ok({ body: resp });
try {
const resp = await client('enhancements.getDataConnections');
return response.ok({ body: resp });
} catch (error) {
if (error.statusCode === 404 || error.statusCode === 400) {
return response.ok({ body: [] });
}
return response.custom({ statusCode: error.statusCode || 500, body: error.message });
}
}
);

Expand Down
Loading
Loading