Ignore unsupported plugin when reporting livestate#5942
Conversation
Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5942 +/- ##
==========================================
- Coverage 28.47% 28.47% -0.01%
==========================================
Files 520 520
Lines 56415 56410 -5
==========================================
- Hits 16063 16060 -3
+ Misses 39086 39084 -2
Partials 1266 1266
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
7b6aac8 to
ab14df5
Compare
t-kikuc
left a comment
There was a problem hiding this comment.
LGTM
When livestatereporter has performance issues, let's improve.
| // TODO: show plugin name | ||
| r.logger.Info("plugin does not support livestate feature") | ||
| continue | ||
| } |
There was a problem hiding this comment.
When needed in the future (not now),
let's remove such plugins from pluginClis, err := r.pluginRegistry.GetPluginClientsByAppConfig(cfg.Spec) by like r.pluginRegistry.markPluginAsLiveStateUnsupported(xxx) to reduce unnecessary gRPC calls.
What this PR does:
Get only livestate-supported plugin clients when flushing the livestate.
Currently, the livestate reporter fails to report when there are some plugins that don't support the livestate feature. (e.g. wait stage).
Livestate reporter calls GetLivestate for each plugin when flushing an app.
But currently, it fails when one of them has an error, including an unimplemented status.
Why we need it:
Which issue(s) this PR fixes:
Follow #5940
Part of #5363
Does this PR introduce a user-facing change?: