-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Builder.For should panic when called twice #1184
Comments
I just ran into the problem that the builder fails if I think it would be the best if |
@alvaroaleman What do you think here? I'm ok having it panic given that's definitely a user error |
#1176 already added an error that is returned on
No. There is only one root type per controller. You have |
Ok, but can we change the builder that it is possible to use it without |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
Stale issues rot after 30d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-contributor-experience at kubernetes/community. |
Rotten issues close after 30d of inactivity. Send feedback to sig-contributor-experience at kubernetes/community. |
@fejta-bot: Closing this issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
I know that a Controller should just control for a single Resource kind, this is an edgy-case.
With the following said
controller-runtime/pkg/builder/controller.go
Lines 65 to 68 in fdc6658
I'm considering
For
a shorthand forWatches
that allows multiple inputscontroller-runtime/pkg/builder/controller.go
Lines 89 to 100 in fdc6658
but that's not true, because I got a single
ForInput
controller-runtime/pkg/builder/controller.go
Lines 69 to 81 in fdc6658
Returning an error would be easier, although breaking the method chaining that is a de-facto standard with the Builder pattern: maybe is better to panic rather than making this non-predictable.
WDYT?
The text was updated successfully, but these errors were encountered: