-
Notifications
You must be signed in to change notification settings - Fork 156
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
Use plugin registry instead of pluginClients and stageBasedPluingsMap in the controller, scheduler, and planner #5472
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5472 +/- ##
==========================================
+ Coverage 26.17% 26.19% +0.01%
==========================================
Files 458 458
Lines 49169 49172 +3
==========================================
+ Hits 12871 12881 +10
+ Misses 35275 35267 -8
- Partials 1023 1024 +1 ☔ View full report in Codecov by Sentry. |
… in the controller, scheduler, and planner Signed-off-by: Yoshiki Fujikane <[email protected]>
47fc867
to
40c6c1e
Compare
FYI, I checked the behavior of the four cases with the patch. diff.patch
It works as I expected. Only
Only
Both
None of
|
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.
Great. I commented about logging.
Signed-off-by: Yoshiki Fujikane <[email protected]>
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.
LGTM
What this PR does:
as title
Why we need it:
Until now, we use
pluginClients
andstageBasedPluingsMap
in thecontroller
,scheduler
,planner
to determine the plugin for the app.However, we implemented the plugin registry in #5467 to determine the plugin used to deploy an app from the app config.
So, we will use it instead of
pluginClients
andstageBasedPluingsMap
from now on.Which issue(s) this PR fixes:
Part of #4980
Follows #5467 #5470
Does this PR introduce a user-facing change?: