Add data2summary check in data summary panel in discover #8716

Qxisylolo · 2024-10-28T04:15:29Z

Description

This pr adds data2summary agent check in data summary panel in discover.
if there is a query assistant agent but not to summary agent then the summary also won’t be displayed

Screenshot

no agent:

local：

data2summary agent

2024-10-28.14.51.26.1.mov

Changelog

feat: adds data2summary agent check in data summary panel in discover.

Check List

Signed-off-by: Qxisylolo <[email protected]>

ruanyl

Approved with a minor comment

ruanyl · 2024-10-30T06:16:06Z

src/plugins/query_enhancements/common/constants.ts

@@ -7,6 +7,7 @@ export const PLUGIN_ID = 'queryEnhancements';
 export const PLUGIN_NAME = 'queryEnhancements';

 export const BASE_API = '/api/enhancements';
+export const BASE_API_AGENT = '/api/assistant';


Nit

Suggested change

export const BASE_API_AGENT = '/api/assistant';

export const BASE_API_ASSISTANT = '/api/assistant';

ruanyl · 2024-10-30T06:30:19Z

src/plugins/query_enhancements/public/query_assist/utils/get_is_summary_agent.ts

+    });
+    return response;
+  } catch (error) {
+    throw error;


btw, I don't see the purpose of this catch because it throws it immediately which is the same as not having the try-catch block.

ok, I see. thank you

Signed-off-by: Qxisylolo <[email protected]>

Hailong-am · 2024-12-19T07:27:02Z

src/plugins/query_enhancements/public/query_assist/utils/get_is_summary_agent.ts

+export async function checkAgentsExist(
+  http: HttpSetup,
+  agentConfigName: string | string[],
+  dataSourceId: string


nit: dataSourceId should be optional

Hailong-am · 2024-12-19T07:30:06Z

src/plugins/query_enhancements/public/query_assist/components/query_assist_summary.tsx

+      if (selectedDataset.current?.dataSource?.id) {
+        try {


we should support datasource not enabled case

Thanks for the comments, updated

We may not need to have if (selectedDataset.current?.dataSource?.id), call await checkAgentsExist() with selectedDataset.current?.dataSource?.id directly should be good. because:

when data source is not enabled, the selectedDataset.dataSource is undefined, in such case, we passed undefined to checkAgentsExist, this will make it to check local cluster

when data source is enabled, but the selected dataset is a local cluster, the selectedDataset.dataSource is undefined as well, in this case, we passed undefined to checkAgentsExist which makes it to call local cluster.

Signed-off-by: Qxisylolo <[email protected]>

ruanyl · 2024-12-25T09:58:36Z

src/plugins/query_enhancements/public/query_assist/utils/get_is_summary_agent.ts

+  queryParams.append('dataSourceId', dataSourceId);
+
+  const response = await http.get(API.AGENT_API.CONFIG_EXISTS, {
+    query: Object.fromEntries(queryParams),


There might be problem to call Object.fromEntries(queryParams), for example:

// if calling append two times queryParams.append('agentConfigName', 'name1') queryParams.append('agentConfigName', 'name2') // this returns {agentConfigName: 'name2'}, which name1 is missing Object.fromEntries(queryParams),

I think you can simply call in this way

const response = await http.get(API.AGENT_API.CONFIG_EXISTS, { query: { agentConfigName, dataSourceId }, });

thanks for the advice，already updated~

Signed-off-by: Qxisylolo <[email protected]>

SuZhou-Joe · 2025-01-03T08:21:09Z

src/plugins/query_enhancements/public/query_assist/components/query_assist_summary.tsx

+        console.error(error);
+      }
+    };
+    fetchSummaryAgent();


Should we call this method only when isEnabledByCapability is true?

Thanks for the reminder, added it

Signed-off-by: Qxisylolo <[email protected]>

* basiclly work Signed-off-by: Qxisylolo <[email protected]> * remove comments Signed-off-by: Qxisylolo <[email protected]> * add tests Signed-off-by: Qxisylolo <[email protected]> * call api Signed-off-by: Qxisylolo <[email protected]> * resolve comments Signed-off-by: Qxisylolo <[email protected]> * add new tests Signed-off-by: Qxisylolo <[email protected]> * update name Signed-off-by: Qxisylolo <[email protected]> * Changeset file for PR #8716 created/updated * resolve new comments Signed-off-by: Qxisylolo <[email protected]> * new resolve comments Signed-off-by: Qxisylolo <[email protected]> * add isEnabledByCapability Signed-off-by: Qxisylolo <[email protected]> --------- Signed-off-by: Qxisylolo <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit d159d75) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…project#8716) * basiclly work Signed-off-by: Qxisylolo <[email protected]> * remove comments Signed-off-by: Qxisylolo <[email protected]> * add tests Signed-off-by: Qxisylolo <[email protected]> * call api Signed-off-by: Qxisylolo <[email protected]> * resolve comments Signed-off-by: Qxisylolo <[email protected]> * add new tests Signed-off-by: Qxisylolo <[email protected]> * update name Signed-off-by: Qxisylolo <[email protected]> * Changeset file for PR opensearch-project#8716 created/updated * resolve new comments Signed-off-by: Qxisylolo <[email protected]> * new resolve comments Signed-off-by: Qxisylolo <[email protected]> * add isEnabledByCapability Signed-off-by: Qxisylolo <[email protected]> --------- Signed-off-by: Qxisylolo <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>

* basiclly work * remove comments * add tests * call api * resolve comments * add new tests * update name * Changeset file for PR #8716 created/updated * resolve new comments * new resolve comments * add isEnabledByCapability --------- (cherry picked from commit d159d75) Signed-off-by: Qxisylolo <[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> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>

github-actions bot added the seasoned-contributor label Oct 28, 2024

opensearch-changeset-bot bot added a commit to Qxisylolo/OpenSearch-Dashboards that referenced this pull request Oct 28, 2024

Changeset file for PR opensearch-project#8716 created/updated

48f82c5

Qxisylolo force-pushed the feat/hide_olly_overlay branch 2 times, most recently from f28a766 to 1edd20d Compare October 28, 2024 04:34

opensearch-changeset-bot bot added a commit to Qxisylolo/OpenSearch-Dashboards that referenced this pull request Oct 28, 2024

Changeset file for PR opensearch-project#8716 created/updated

bfdd348

Qxisylolo force-pushed the feat/hide_olly_overlay branch from bfdd348 to f48615a Compare October 28, 2024 06:50

opensearch-changeset-bot bot added a commit to Qxisylolo/OpenSearch-Dashboards that referenced this pull request Oct 28, 2024

Changeset file for PR opensearch-project#8716 created/updated

51f1919

Qxisylolo marked this pull request as ready for review October 28, 2024 06:55

add new tests

38e53bd

Signed-off-by: Qxisylolo <[email protected]>

Qxisylolo force-pushed the feat/hide_olly_overlay branch from 042044a to 38e53bd Compare October 30, 2024 04:08

ruanyl previously approved these changes Oct 30, 2024

View reviewed changes

ruanyl reviewed Oct 30, 2024

View reviewed changes

opensearch-changeset-bot bot added a commit to Qxisylolo/OpenSearch-Dashboards that referenced this pull request Oct 30, 2024

Changeset file for PR opensearch-project#8716 created/updated

afc53a7

opensearch-changeset-bot bot dismissed ruanyl’s stale review via afc53a7 October 30, 2024 06:33

opensearch-changeset-bot bot added a commit to Qxisylolo/OpenSearch-Dashboards that referenced this pull request Oct 30, 2024

Changeset file for PR opensearch-project#8716 created/updated

788f842

update name

4fcec84

Signed-off-by: Qxisylolo <[email protected]>

Qxisylolo force-pushed the feat/hide_olly_overlay branch from 788f842 to 4fcec84 Compare October 30, 2024 07:05

Changeset file for PR opensearch-project#8716 created/updated

4a12194

ruanyl previously approved these changes Oct 30, 2024

View reviewed changes

ruanyl added the v2.19.0 label Oct 31, 2024

Hailong-am reviewed Dec 19, 2024

View reviewed changes

resolve new comments

94dd3a6

Signed-off-by: Qxisylolo <[email protected]>

Qxisylolo dismissed ruanyl’s stale review via 94dd3a6 December 25, 2024 05:47

Merge branch 'main' into feat/hide_olly_overlay

0bd2c64

ruanyl reviewed Dec 25, 2024

View reviewed changes

Qxisylolo added 2 commits December 26, 2024 11:37

new resolve comments

1a8393b

Signed-off-by: Qxisylolo <[email protected]>

Merge branch 'main' into feat/hide_olly_overlay

39dd8f5

ruanyl previously approved these changes Dec 26, 2024

View reviewed changes

SuZhou-Joe reviewed Jan 3, 2025

View reviewed changes

add isEnabledByCapability

b5917ba

Signed-off-by: Qxisylolo <[email protected]>

Qxisylolo dismissed ruanyl’s stale review via b5917ba January 3, 2025 09:32

SuZhou-Joe approved these changes Jan 6, 2025

View reviewed changes

ruanyl approved these changes Jan 17, 2025

View reviewed changes

ruanyl merged commit d159d75 into opensearch-project:main Jan 17, 2025
69 checks passed

opensearch-trigger-bot bot mentioned this pull request Jan 17, 2025

[Backport 2.x] Add data2summary check in data summary panel in discover #9204

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add data2summary check in data summary panel in discover #8716

Add data2summary check in data summary panel in discover #8716

Qxisylolo commented Oct 28, 2024 •

edited

Loading

ruanyl left a comment

ruanyl Oct 30, 2024

ruanyl Oct 30, 2024

Qxisylolo Oct 30, 2024

Hailong-am Dec 19, 2024

Hailong-am Dec 19, 2024

Qxisylolo Dec 25, 2024

ruanyl Dec 25, 2024

ruanyl Dec 25, 2024 •

edited

Loading

Qxisylolo Dec 26, 2024

SuZhou-Joe Jan 3, 2025

Qxisylolo Jan 3, 2025

	export const BASE_API_AGENT = '/api/assistant';
	export const BASE_API_ASSISTANT = '/api/assistant';

Add data2summary check in data summary panel in discover #8716

Add data2summary check in data summary panel in discover #8716

Conversation

Qxisylolo commented Oct 28, 2024 • edited Loading

Description

Screenshot

Changelog

Check List

ruanyl 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruanyl Dec 25, 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

Qxisylolo commented Oct 28, 2024 •

edited

Loading

ruanyl Dec 25, 2024 •

edited

Loading