✨ introduce LabelChangedPredicate to trigger event on label change #1315
Conversation
Signed-off-by: Bruce Ma <[email protected]>
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/L
mars1024
changed the title
mars1024
changed the title
| // This will be helpful when object's labels is carrying some extra specification information beyond object's spec, | ||
| // and the controller will be triggered if any valid spec change (not only in spec, but also in labels) happens. | ||
| type LabelChangedPredicate struct { | ||
| Funcs |
There was a problem hiding this comment.
Can you use Funcs.UpdateFunc rather than overwriting it?
There was a problem hiding this comment.
Sorry, maybe I didn't catch your point, I'm just doing the same way like ResourceVersionChangedPredicate and GenerationChangedPredicate, overriding the Update method and inherit the others.
If we want to assign Funcs.UpdateFunc, another constructing function will be introduced for this, and I think maybe it's not easy for using.
There was a problem hiding this comment.
Ah that's how you came up with it. From a user perspective it is just calling a constructor rather than using a type :)
The reason I think it's slightly better is that it might be confusing to have an embedded UpdateFunc described as being called for Update events that does not actually get called for Update events :)
There was a problem hiding this comment.
:) Got your point, but your suggestion is keeping same as the other predicates or change all the predicates to a new way (use a constructor instead of stating a type), but I think the second way maybe a break change for some "legacy" users.
Or maybe we could just use the constructing way for the new LabelChangedPredicate?
| // This will be helpful when object's labels is carrying some extra specification information beyond object's spec, | ||
| // and the controller will be triggered if any valid spec change (not only in spec, but also in labels) happens. | ||
| type LabelChangedPredicate struct { | ||
| Funcs |
There was a problem hiding this comment.
Ah that's how you came up with it. From a user perspective it is just calling a constructor rather than using a type :)
The reason I think it's slightly better is that it might be confusing to have an embedded UpdateFunc described as being called for Update events that does not actually get called for Update events :)
pkg/predicate/predicate_test.go
Outdated
| @@ -14,6 +14,7 @@ See the License for the specific language governing permissions and | |||
| limitations under the License. | |||
| */ | |||
|
|
|||
| //nolint:dupl | |||
There was a problem hiding this comment.
Why is this needed? Can we add an explanation comment for posterity and set it on a more granular level than on the whole package?
There was a problem hiding this comment.
comments is added and level is set to single function from whole package
mars1024
force-pushed
the
feat/label-change-predicate
branch
5 times, most recently
from
9a1406d to
4b527ac
Compare
Signed-off-by: Bruce Ma <[email protected]>
mars1024
force-pushed
the
feat/label-change-predicate
branch
from
4b527ac to
52c9e6f
Compare
|
/retest |
|
friendly ping @alvaroaleman , could you help to promote this PR? I think all comments have been addressed. |
k8s-ci-robot
added
the
tide/merge-method-squash
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alvaroaleman, mars1024 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 |
k8s-ci-robot
added
the
approved
|
/retest |
…ubernetes-sigs#1315) * introduce LabelChangedPredicate to trigger event on label change Signed-off-by: Bruce Ma <[email protected]> * disable duplication linter on predicate tests Signed-off-by: Bruce Ma <[email protected]>
Signed-off-by: Bruce Ma [email protected]
This is a non-breaking feature ✨ .
Just as the title, a
LabelChangedPredicateis introduced to trigger event on label change, if some extra specification information is carried through object's label, this predicate function will be useful.