Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Fix detector list infinite loading on cluster initialization #177

Merged

Conversation

ohltyler
Copy link
Contributor

@ohltyler ohltyler commented May 22, 2020

Issue #, if available: #176

Description of changes:

On cluster initialization (when .opendistro-anomaly-detectors index doesn't exist yet) there is an uncaught exception on the detector list page if a user tries changing any filters / searching / sorting etc., leaving the page in an infinite loading state.
This fixes that by fixing the error handling when making the backend call.

Before:
Screen Shot 2020-05-22 at 10 12 35 AM

After:
Screen Shot 2020-05-22 at 10 11 58 AM

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Comment on lines -141 to +156
dispatch(getDetectorList(GET_ALL_DETECTORS_QUERY_PARAMS));
const getUpdatedDetectors = async () => {
try {
await dispatch(getDetectorList(GET_ALL_DETECTORS_QUERY_PARAMS));
} catch (error) {
console.log('Error is found during getting detector list', error);
setIsLoadingFinalDetectors(false);
}
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the only change; the other changes are auto-formatting changes. Somehow my local dev environment stopped auto-formatting on save at some point, fixed it now.

Copy link
Contributor

@ylwu-amzn ylwu-amzn left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the fix.

await dispatch(getDetectorList(GET_ALL_DETECTORS_QUERY_PARAMS));
} catch (error) {
console.log('Error is found during getting detector list', error);
setIsLoadingFinalDetectors(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

should we movesetIsLoadingFinalDetectors(false); to finally? looks like only when exception is caught, isLoading can be set to false

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's the behavior I want here. It should only be set to false if there was an error. If successful and the detectors are retrieved, it should be set to true until the final set of sorted detectors to display are finished here.

Note that isLoadingFinalDetectors is only related to sorting/filtering in the frontend. isRequestingFromES handles loading from the backend.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it.

@ohltyler ohltyler merged commit 4ccde12 into opendistro-for-elasticsearch:master May 22, 2020
@ohltyler ohltyler deleted the infiniteLoading branch May 22, 2020 18:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Infinite loading on detector list page on cluster initialization
3 participants