Skip to content
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

Handlers should default to LowPriorityWhenUnchanged without a wrapper #3105

Open
alvaroaleman opened this issue Feb 7, 2025 · 4 comments · Fixed by #3111 or #3152
Open

Handlers should default to LowPriorityWhenUnchanged without a wrapper #3105

alvaroaleman opened this issue Feb 7, 2025 · 4 comments · Fixed by #3111 or #3152

Comments

@alvaroaleman
Copy link
Member

When we implemented the logic in the handlers to lower the priority for events where the object didn't change, we did that with a wrapper as that can be re-used.

Unfortunately, this requires everyone to remember to use this wrapper, which is extremely easy to forget:

handler.WithLowPriorityWhenUnchanged(&handler.TypedEnqueueRequestForObject[*my.CRD]{})

We should instead implement this in the handlers themselves and default to true to avoid the need for this.

@sbueringer
Copy link
Member

/reopen

Not entirely done yet

@k8s-ci-robot k8s-ci-robot reopened this Mar 10, 2025
@k8s-ci-robot
Copy link
Contributor

@sbueringer: Reopened this issue.

In response to this:

/reopen

Not entirely done yet

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-sigs/prow repository.

@sbueringer
Copy link
Member

/reopen

@alvaroaleman Do we have to modify more handlers?

  • EnqueueRequestForObject
  • TypedEnqueueRequestForOwner[client.Object]

+ Should we stop wrapping the handlers with WithLowPriorityWhenUnchanged in pkg/builder/controller.go? Seems redundant

@k8s-ci-robot k8s-ci-robot reopened this Mar 13, 2025
@k8s-ci-robot
Copy link
Contributor

@sbueringer: Reopened this issue.

In response to this:

/reopen

@alvaroaleman Do we have to modify more handlers?

  • EnqueueRequestForObject
  • TypedEnqueueRequestForOwner[client.Object]

+ Should we stop wrapping the handlers with WithLowPriorityWhenUnchanged in pkg/builder/controller.go? Seems redundant

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-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants