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

[Bug fix] Metrics datasource #2242

Merged
merged 3 commits into from
Nov 5, 2024

Conversation

TackAdam
Copy link
Collaborator

@TackAdam TackAdam commented Nov 1, 2024

Description

While datasource is enabled, set initial condition to null and wait to render page until the selection/default is registered.

Currently if a localhost is not defined the call will trigger the search on a non valid datasource.
NoLocalHost

This change prevents the search call from being triggered until the datasource has been read.

MetricsSearch.mov

Issues Resolved

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • Commits are signed per the DCO using --signoff

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.

@TackAdam TackAdam added backport 2.x bug Something isn't working mds-2.18.0 labels Nov 1, 2024
@TackAdam TackAdam marked this pull request as ready for review November 4, 2024 18:09
@@ -95,6 +97,11 @@ export const Sidebar = ({
}, [selectedMetrics, selectedMetricsIds, dataSourceMDSId]);

useEffect(() => {
// If data source is enabled but is still null prevent the invalid call
if (dataSourceEnabled && dataSourceMDSId === null) {
Copy link
Member

Choose a reason for hiding this comment

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

if local cluster is selected, what would dataSourceMDSId be?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Local cluster is just an empty string "" when selected it get sets and is no longer null

@@ -77,7 +77,7 @@ export const Home = ({
const dispatch = useDispatch();

const onSelectedDataSource = async (dataSources: DataSourceOption[]) => {
const id = dataSources[0] ? dataSources[0].id : '';
const id = dataSources[0] ? dataSources[0].id : null;
Copy link
Member

Choose a reason for hiding this comment

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

Only one question: What's the condition when dataSources[0] will be undefined and we have to set the id as null?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The only condition this should be triggered is when a workspace is created and its datasource is removed putting it in a state where it has no data sources available at all. In this case metrics won't work and we don't want to display options.
We should probably have an empty state for this such as " No connected data source with redirection to associate one".

@TackAdam TackAdam merged commit 397a867 into opensearch-project:main Nov 5, 2024
11 of 18 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Nov 5, 2024
* set default datasource to null and prevent rendering until it is set

Signed-off-by: Adam Tackett <[email protected]>

* prevent invalid api call

Signed-off-by: Adam Tackett <[email protected]>

---------

Signed-off-by: Adam Tackett <[email protected]>
Co-authored-by: Adam Tackett <[email protected]>
(cherry picked from commit 397a867)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
ps48 pushed a commit that referenced this pull request Nov 6, 2024
* set default datasource to null and prevent rendering until it is set



* prevent invalid api call



---------



(cherry picked from commit 397a867)

Signed-off-by: Adam Tackett <[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: Adam Tackett <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x bug Something isn't working mds-2.18.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants