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

Fix incorrect callback function #1443

Merged

Conversation

upsilon2gamma
Copy link
Contributor

@upsilon2gamma upsilon2gamma commented Nov 29, 2023

Fixes #1442

According to JQuery doc (https://api.jquery.com/jQuery.ajax/), complete function will be called whether the ajax request succeeds or fails. As a result, _refreshClusterStatus will set two new scheduled calls if the ajax request gets an error response (the first one by error and the second one by complete).

If _refreshClusterStatus keeps getting error responses (e.g. token has expired, the ajax request always gets 304 response), it will generate a huge number of requests in 10min (doubles every 30s)

Copy link
Contributor

@adejanovski adejanovski left a comment

Choose a reason for hiding this comment

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

Hi @upsilon2gamma,

you're right, in case of error there's some duplication of timers that will happen.
May I suggest that we keep the complete: callback and modify the error: one by removing the this.component.setState(... line, keeping just the console logging?
This way we'll remove some unnecessary lines of code.

Thanks!

https://api.jquery.com/jQuery.ajax/
According to JQuery doc, `complete` will be called whether the ajax
request succeeds or fails. As a result, `_refreshClusterStatus` will
generates two new scheduled calls if the ajax request gets an error
response (one by `error` and one by `complete`).
If `_refreshClusterStatus` keeps getting error responses (e.g. token
has expired), it will generate a huge number of requests (doubles every
30s)
@upsilon2gamma
Copy link
Contributor Author

@adejanovski Updated

Copy link
Contributor

@adejanovski adejanovski left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@adejanovski adejanovski merged commit ec1f16e into thelastpickle:master Dec 13, 2023
1 check 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: webui will generate a huge number of requests if _refreshClusterStatus keep getting error responses
2 participants