You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Having update/patch methods with filtering/caching updates in context #3136
Was thinking more about this, and I would stick with the Utility class, basically the only benefit would be that we could remove the context parameter from those methods; But would pollute context quite a bit with lot of utility methods. If you feel that still needed pls create a PR so we can discuss it based on code and changes. @metacosm@xstefank
what about then allowing to create an "instance" of utility class with the current context that the user can save in their reconciler?
Would be a bit ugly, especially since it is kinda anti pattern for utility classes, but mainly since that there are methods which would have be static anyways, so we would mix those.
But I think we should ask the question what is the problem we are solving. Passing 2 params to a method is not extreme at all (even for Uncle Bob standards); Also it is a matter of definition a bit how we want to model the domain, and what meant with the current designs:
In Context we have access to actual contextual state relevant for the reconciliation
ReconcileUtils will contain all the possible relevant actions for the reconciliation
I though this is a good enough mental model, without too much technical downside :)
what about then allowing to create an "instance" of utility class with the current context that the user can save in their reconciler?
Actually, this would make a lot of sense to me: all the methods could become regular methods instead of static one and the instance would have a local field holding the current context so it wouldn't have to be passed around to all the static methods. Plus, this would help with discovery since it would be accessible from the Context…
I see, maybe I missunderstood how do you mean it, so should be this a separate class and not methods directly in Context. Let's say we extract those methods into a KubernetesClientFacade (or something similar) that will hold the context - and accssible from context. This makes sense to me also.
Would you move there also the addFinalizer (and alternatives) methods, those are just for quite special cases?
Hi @metacosm@xstefank added a quick prototype (just the API now), please take a look, and let me know if this is how did you mean it.
And thx for the good discussion around this!
changed the title [-]Having update/patch methods with filtering/caching updates in context (?)[/-][+]Having update/patch methods with filtering/caching updates in context[/+]on Feb 2, 2026
Activity
csviri commentedon Jan 27, 2026
cc @metacosm @xstefank
csviri commentedon Jan 27, 2026
TODO discuss on community meeting.
csviri commentedon Jan 29, 2026
Was thinking more about this, and I would stick with the Utility class, basically the only benefit would be that we could remove the context parameter from those methods; But would pollute context quite a bit with lot of utility methods. If you feel that still needed pls create a PR so we can discuss it based on code and changes. @metacosm @xstefank
xstefank commentedon Jan 30, 2026
what about then allowing to create an "instance" of utility class with the current context that the user can save in their reconciler?
csviri commentedon Jan 30, 2026
Would be a bit ugly, especially since it is kinda anti pattern for utility classes, but mainly since that there are methods which would have be static anyways, so we would mix those.
But I think we should ask the question what is the problem we are solving. Passing 2 params to a method is not extreme at all (even for Uncle Bob standards); Also it is a matter of definition a bit how we want to model the domain, and what meant with the current designs:
I though this is a good enough mental model, without too much technical downside :)
metacosm commentedon Jan 30, 2026
Actually, this would make a lot of sense to me: all the methods could become regular methods instead of static one and the instance would have a local field holding the current context so it wouldn't have to be passed around to all the static methods. Plus, this would help with discovery since it would be accessible from the
Context…csviri commentedon Jan 30, 2026
I see, maybe I missunderstood how do you mean it, so should be this a separate class and not methods directly in Context. Let's say we extract those methods into a
KubernetesClientFacade(or something similar) that will hold the context - and accssible from context. This makes sense to me also.Would you move there also the
addFinalizer(and alternatives) methods, those are just for quite special cases?csviri commentedon Jan 30, 2026
Let me do a quick prototype how it would look like.
csviri commentedon Jan 30, 2026
Hi @metacosm @xstefank added a quick prototype (just the API now), please take a look, and let me know if this is how did you mean it.
And thx for the good discussion around this!
[-]Having update/patch methods with filtering/caching updates in context (?)[/-][+]Having update/patch methods with filtering/caching updates in context[/+]