-
Notifications
You must be signed in to change notification settings - Fork 2k
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
UI: Retry stats requests #4195
UI: Retry stats requests #4195
Conversation
JSON.stringify({ ResourceUsage: generateResources() }), | ||
JSON.stringify({ ResourceUsage: generateResources() }), | ||
null, | ||
'<Not>Valid JSON</Not>', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nicely done - great job with the tests too, I know the polling stuff can be gnarly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seconding the congrats on tests for this component, nice stuff.
|
||
// Disable polling to stop the EC task in the component | ||
if (currentFrame >= frames.length) { | ||
component.set('enablePolling', false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't seen this idea before, pretty neat. Did you consider using cancelTimers
and prefer this instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran through a lot of combinations, but not the cancelTimers
from the pretender handler combination.
Sounds like it would work, but since I already have this enablePolling
hook to disable polling in acceptance tests, I'm not sure what the benefit would be.
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Before: If the alloc stats poller encountered an error, it gave up
After: The alloc stats poller will show the warning signs when encountering errors, but will continue to poll.