From d4f95c906974b6e65fb2298f912b4327604102c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A1roly=20Szak=C3=A1ly?= Date: Mon, 9 Sep 2024 21:07:56 +0200 Subject: [PATCH] [Feature] Add the ability to configure record limit/count from the UI (#395) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(components/report-settings): add field for variable record limits Signed-off-by: Károly Szakály * fix(components/report-settings): set type for event target Signed-off-by: Károly Szakály * feat(components/report-settings): load existing record limit in edit mode Signed-off-by: Károly Szakály * feat(components/report-definition-details): show record limit on report definition details page Signed-off-by: Károly Szakály * feat(components/report-definition-details): omit record limit for non saved search sources Signed-off-by: Károly Szakály * feat(components/report-details): show record limit on report details page Signed-off-by: Károly Szakály * tests(cypress): add `force: true` to name input Signed-off-by: Károly Szakály * tests(jest/snapshots): update snapshots Signed-off-by: Károly Szakály * feat(components/report-settings): add proper i18n label for record limit input Signed-off-by: Károly Szakály * Update release notes Signed-off-by: Simeon Widdis * Add potential OOM warning for large record limits Signed-off-by: Simeon Widdis * Make memory warning more conservative based on original limit Signed-off-by: Simeon Widdis --------- Signed-off-by: Károly Szakály Signed-off-by: Simeon Widdis Co-authored-by: Simeon Widdis --- .cypress/integration/02-edit.spec.ts | 2 +- .../report_definition_details.tsx | 13 +++++ .../main/report_details/report_details.tsx | 15 +++++ .../report_settings/report_settings.tsx | 56 +++++++++++++++++-- ...boards-reporting.release-notes-2.17.0.0.md | 1 + 5 files changed, 81 insertions(+), 6 deletions(-) diff --git a/.cypress/integration/02-edit.spec.ts b/.cypress/integration/02-edit.spec.ts index a87e77d4..23a48b56 100644 --- a/.cypress/integration/02-edit.spec.ts +++ b/.cypress/integration/02-edit.spec.ts @@ -24,7 +24,7 @@ describe('Cypress', () => { cy.wait(1000); // update the report name - cy.get('#reportSettingsName').type(' update name'); + cy.get('#reportSettingsName').type(' update name', { force: true }); // update report description cy.get('#reportSettingsDescription').type(' update description'); diff --git a/public/components/main/report_definition_details/report_definition_details.tsx b/public/components/main/report_definition_details/report_definition_details.tsx index 0963d594..b895dba0 100644 --- a/public/components/main/report_definition_details/report_definition_details.tsx +++ b/public/components/main/report_definition_details/report_definition_details.tsx @@ -45,6 +45,7 @@ interface ReportDefinitionDetails { created: string; lastUpdated: string; source: string; + recordLimit: number | undefined; timePeriod: string; fileFormat: string; status: string | undefined; @@ -63,6 +64,7 @@ export function ReportDefinitionDetails(props: { match?: any; setBreadcrumbs?: a created: '', lastUpdated: '', source: '', + recordLimit: 0, timePeriod: '', fileFormat: '', status: '', @@ -417,6 +419,10 @@ export function ReportDefinitionDetails(props: { match?: any; setBreadcrumbs?: a created: displayCreatedDate, lastUpdated: displayUpdatedDate, source: reportParams.report_source, + recordLimit: + reportParams.report_source != 'Saved search' + ? `\u2014` + : reportParams.core_params.limit, baseUrl: baseUrl, // TODO: need better display timePeriod: moment.duration(timeDuration).humanize(), @@ -778,6 +784,13 @@ export function ReportDefinitionDetails(props: { match?: any; setBreadcrumbs?: a )} reportDetailsComponentContent={sourceURL(reportDefinitionDetails)} /> + + + + { + setSavedSearchRecordLimit(e.target.value); + + reportDefinitionRequest.report_params.core_params.limit = e.target.value; + }; + const handleNotebooksSelect = (e) => { setNotebooksSourceSelect(e); let fromInContext = false; @@ -592,6 +604,13 @@ export function ReportSettings(props: ReportSettingProps) { reportDefinitionRequest.report_params.report_source = reportSource; } }); + + if (reportSource == REPORT_SOURCE_TYPES.savedSearch) { + setSavedSearchRecordLimit( + response.report_definition.report_params.core_params.limit + ); + } + setDefaultFileFormat( response.report_definition.report_params.core_params.report_format ); @@ -666,8 +685,11 @@ export function ReportSettings(props: ReportSettingProps) { await httpClientProps .get('../api/observability/notebooks/') .catch((error: any) => { - console.error('error fetching notebooks, retrying with legacy api', error) - return httpClientProps.get('../api/notebooks/') + console.error( + 'error fetching notebooks, retrying with legacy api', + error + ); + return httpClientProps.get('../api/notebooks/'); }) .then(async (response: any) => { let notebooksOptions = getNotebooksOptions(response.data); @@ -704,7 +726,7 @@ export function ReportSettings(props: ReportSettingProps) { reportSourceId === 'dashboardReportSource' ? (
+ 10000 ? ( + + Generating + very large reports can cause memory issues. + + ) : ( + '' + ) + } + > + + +
) : null; diff --git a/release-notes/opensearch-dashboards-reporting.release-notes-2.17.0.0.md b/release-notes/opensearch-dashboards-reporting.release-notes-2.17.0.0.md index 7eeddf75..02624011 100644 --- a/release-notes/opensearch-dashboards-reporting.release-notes-2.17.0.0.md +++ b/release-notes/opensearch-dashboards-reporting.release-notes-2.17.0.0.md @@ -3,6 +3,7 @@ Compatible with OpenSearch and OpenSearch Dashboards Version 2.17.0 ### Enhancements +* [Feature] Add the ability to configure record limit/count from the UI ([#395](https://github.com/opensearch-project/dashboards-reporting/pull/395)) * Use smaller and compressed varients of buttons and form components ([#398](https://github.com/opensearch-project/dashboards-reporting/pull/398)) * [Enhancement] De-register reporting when MDS is enabled ([#411](https://github.com/opensearch-project/dashboards-reporting/pull/411))