Update status when summary is updated #128769
Conversation
nchaulet
added
bug
|
|
||
| this.pluginData[plugin].reportedStatus = reportedStatus; | ||
| this.pluginStatus[plugin] = reportedStatus; | ||
|
|
||
| if (reportedStatus.level !== previousReportedLevel) { | ||
| if ( | ||
| reportedStatus.level !== previousReportedLevel || |
There was a problem hiding this comment.
should we compare the whole reportedStatus to previoudReportedStatus here?
There was a problem hiding this comment.
It might make us less prone to breakage if compare the whole status object here, yes. What does this object look like? Is it simple enough to do a shallow equality check here?
nchaulet
added
release_note:skip
|
|
||
| this.pluginData[plugin].reportedStatus = reportedStatus; | ||
| this.pluginStatus[plugin] = reportedStatus; | ||
|
|
||
| if (reportedStatus.level !== previousReportedLevel) { | ||
| if ( | ||
| reportedStatus.level !== previousReportedLevel || |
There was a problem hiding this comment.
It might make us less prone to breakage if compare the whole status object here, yes. What does this object look like? Is it simple enough to do a shallow equality check here?
There was a problem hiding this comment.
The changes LGTM. I'll rely on @gsoldevila to confirm though.
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: |
nchaulet
added
v8.3.0
auto-backport
There was a problem hiding this comment.
LGTM! Thank you @nchaulet and @gsoldevila for your changes!
(cherry picked from commit abe70c5)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…ucing heap size) (#128324) (#129507) * Rewrite plugin_status logic limiting usage of Observables (reducing heap size) (#128324) * WIP * Fix behavior when no plugins are defined * Remove unused import, reduce debounce times * Fix startup behavior * Misc improvements following PR comments * Fix plugin_status UTs * Code cleanup + enhancements * Remove fixed FIXME (cherry picked from commit a645545) * Add extra tests from #128769 * Fix linting issues (older typescript version?) * Fix TS version issues
Summary
Related to #127075
In #128324 the status feature was rewritten causing status to not be updated if the level do not change, Fleet update the status
summarywithout updating the level.That PR fix that regression, should we compare the whole status and previous status if other fields are updated?