Skip to content

WithLowPriorityWhenUnchanged only sets priority for Add #3171

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

Open
1 task
sbueringer opened this issue Mar 21, 2025 · 4 comments
Open
1 task

WithLowPriorityWhenUnchanged only sets priority for Add #3171

sbueringer opened this issue Mar 21, 2025 · 4 comments

Comments

@sbueringer
Copy link
Member

sbueringer commented Mar 21, 2025

I took a closer look at workqueueWithCustomAddFunc. It currently only overwrites the Add func. Would it also make sense to overwrite AddRateLimited, AddAfter and AddWithOpts?

For AddWithOpts ideally only set priority if not already set, but atm we can't differentiate between priority intentionally set to 0 and the zero value, because Priority in AddOpts is not a pointer (probably it should be).

I know that for our builtin event handler we only have to overwrite Add. But I assume if folks use WithLowPriorityWhenUnchanged they would probably expect that it works for all Add funcs.

Tasks:

  • Consider to make AddOpts.Priority a pointer so we can differentiate between not set and priority 0 (the zero value)
@sbueringer
Copy link
Member Author

cc @alvaroaleman

@alvaroaleman
Copy link
Member

Seems okay to me but I think its pretty uncommon to implement a full handler rather than using handler.Funcs or such

@sbueringer
Copy link
Member Author

If I'm not missing anything even if you use handler.Funcs you would implement a func like this: func(context.Context, event.TypedCreateEvent[object], workqueue.TypedRateLimitingInterface[request])

The workqueue.TypedRateLimitingInterface[request] parameter then provides all Add* funcs, but only Addwill set the low priority

@sbueringer
Copy link
Member Author

Note to myself. When looking into this we might also want to make AddOpts.Priority a pointer so we can differentiate between not set and priority 0 (the zero value)

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

No branches or pull requests

2 participants