-
Notifications
You must be signed in to change notification settings - Fork 43
handler,predicate: pause object reconciliation by annotation #60
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
handler,predicate: pause object reconciliation by annotation #60
Conversation
The tests added here break other handler tests because they share the same metrics registry. If this POC is accepted, the author (me or someone else) will re-write the tests so they do not affect each other. |
Pull Request Test Coverage Report for Build 1049826637
💛 - Coveralls |
// the controller will see events from these objects again. | ||
type filter struct { | ||
key string | ||
ret bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-blocking thought:
It might be generally useful to add a Not(in predicate.Predicate) predicate.Predicate
wrapper in C-R that inverts the logic of the input predicate.
If that existed, would you need the ret
field?
I alluded to this in the C-R PR where I added the Or
and And
wrappers here, but I never added it because there hasn't been a use case for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you'd need ret
here in that case. Given that operator-lib is pre v1.0, are you ok with adding this as-is now then removing if Not
is added to c-r?
@joelanford given that Falsy/Truthy filters are complements, yes |
by event handler or predicate via an annotation with key string key, respectively internal/annotation: add generic library for creating predicates and event handlers for arbitrary annotation keys Signed-off-by: Eric Stroczynski <[email protected]>
Signed-off-by: Eric Stroczynski <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
Not sure why it's not marking this automatically approved. @jmrodri can you |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: varshaprasad96 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description of the change: added
NewPause(key)
to handler and predicate packages that returns an EventHandler and Predicate, respectively, to pause object reconciliation on objects with the<key>: true
annotation.Motivation for the change: something along the lines of operator-framework/operator-sdk#3418.
/kind feature