WIP refactor(controller): move provider construction to provider/factory package#6269
WIP refactor(controller): move provider construction to provider/factory package#6269ivankatliarchuk wants to merge 2 commits intokubernetes-sigs:masterfrom
Conversation
…package Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Pull Request Test Coverage Report for Build 23044379392Details
💛 - Coveralls |
…package Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
|
@ivankatliarchuk 🤔 I'm not sure whether I see any real benefits here. It's slightly better, yes. Wdyt about implementing a registry pattern in this factory? Something like: var providers = make(map[string]BuilderFunc)
func Register(name string, builder BuilderFunc) {
providers[name] = builder
}Each provider package can then call |
|
Well I do agree. There is a PR #6133 that does exactly that, not a registry but with service‑locator style factory. My view is, we first move it away - then find a pattern. I could do it here, not a problem. At the moment, because this provider builder is in controller, we could not reuse provider builder #6158 and test it end-2-end. Your call. |
^ This is I'm not sure. Basically it will allow to reassing at runtime providers. |
|
/retitle WIP refactor(controller): move provider construction to provider/factory package |
|
no longer required |
What does it do ?
Motivation
The controller's buildProvider mixing orchestration concerns with provider-specific construction details (provider construction belongs in the provider tree, not the controller). The existing TODOs in the code already called for moving this logic to the provider package
More