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.

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

Make sure API configuration changes are reflected on the UI #28

Merged
merged 3 commits into from
Nov 12, 2024

Conversation

deshsidd
Copy link
Collaborator

@deshsidd deshsidd commented Nov 12, 2024

Description

This is a bug fix where the API configuration changes are not reflected on the UI on page refresh.

If window_size is not passed while enabling a metric_type the error causes the configuration changes to not display on the UI for the specific type.

TopNQueries.tsx:174 Failed to retrieve settings: TypeError: Cannot read properties of undefined (reading 'match')
    at TopNQueries.tsx:165:1

Fixed by checking for window_size before trying to parse it. If window_size does not exist, use the default value 1m.

Testing

Tested on local by deploying OS with query insights and OSD with query insights dashboards. Verified that updating the settings via API is now reflected when we refresh the UI page.

Issues Resolved

Closes: #27

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@deshsidd deshsidd changed the title Make sure configuration changes refreshed on page load Make sure configuration changes are refreshed on page load Nov 12, 2024
@deshsidd deshsidd changed the title Make sure configuration changes are refreshed on page load Update configuration on page reload Nov 12, 2024
Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com>
@deshsidd deshsidd force-pushed the sid/api-config-change-bug branch from fd67fb9 to b3a38ba Compare November 12, 2024 03:37
@kkhatua kkhatua requested a review from derek-ho November 12, 2024 16:51
@derek-ho
Copy link

I think the issue is actually with not passing in window_size with memory enablement. Error message in console:

TopNQueries.tsx:174 Failed to retrieve settings: TypeError: Cannot read properties of undefined (reading 'match')
    at TopNQueries.tsx:165:1

Which is pointing to https://github.com/opensearch-project/query-insights-dashboards/blob/main/public/pages/TopNQueries/TopNQueries.tsx#L205. If this window_size is not passed, it errors out before setting enabled as true.

This reverts commit b3a38ba.

Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com>
Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com>
@deshsidd deshsidd changed the title Update configuration on page reload Make sure API configuration changes are reflected on the UI Nov 12, 2024
@dzane17 dzane17 merged commit 1e70dc6 into main Nov 12, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Configuration changes via API not reflected on dashboard
3 participants