⚠️ Source, Event, Predicate, Handler: Add generics support #2783
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,38 +18,55 @@ package event | |
|
|
||
| import "sigs.k8s.io/controller-runtime/pkg/client" | ||
|
|
||
| // CreateEvent is an event where a Kubernetes object was created. CreateEvent should be generated | ||
| // by a source.Source and transformed into a reconcile.Request by a handler.EventHandler. | ||
| type CreateEvent = TypedCreateEvent[client.Object] | ||
|
|
||
| // UpdateEvent is an event where a Kubernetes object was updated. UpdateEvent should be generated | ||
| // by a source.Source and transformed into a reconcile.Request by an handler.EventHandler. | ||
| type UpdateEvent = TypedUpdateEvent[client.Object] | ||
|
|
||
| // DeleteEvent is an event where a Kubernetes object was deleted. DeleteEvent should be generated | ||
| // by a source.Source and transformed into a reconcile.Request by an handler.EventHandler. | ||
| type DeleteEvent = TypedDeleteEvent[client.Object] | ||
|
|
||
| // GenericEvent is an event where the operation type is unknown (e.g. polling or event originating outside the cluster). | ||
| // GenericEvent should be generated by a source.Source and transformed into a reconcile.Request by an | ||
| // handler.EventHandler. | ||
| type GenericEvent = TypedGenericEvent[client.Object] | ||
|
|
||
| // TypedCreateEvent is an event where a Kubernetes object was created. TypedCreateEvent should be generated | ||
| // by a source.Source and transformed into a reconcile.Request by an handler.TypedEventHandler. | ||
| type TypedCreateEvent[T any] struct { | ||
| // Object is the object from the event | ||
| Object T | ||
| } | ||
|
|
||
| // TypedUpdateEvent is an event where a Kubernetes object was updated. TypedUpdateEvent should be generated | ||
| // by a source.Source and transformed into a reconcile.Request by an handler.TypedEventHandler. | ||
| type TypedUpdateEvent[T any] struct { | ||
|
There was a problem hiding this comment. It seems to me that having a typed EventHandler here is not as convenient as to have a method which will infer the type of the object. Client-go works with 2 interfaces under the hood, passed in a method if I’m not mistaken. Can this be implemented this way but with generics? There was a problem hiding this comment. You mean a constructor for events and unexporting the current types? That would be a breaking change. There was a problem hiding this comment. Rather leave Event structures for non generic types without any change, and introduce a new handler for generic types. I did it this way: https://github.com/Danil-Grigorev/controller-runtime/blob/4d770047de3a10110f5ded3bce46c178d68d0500/pkg/handler/eventhandler.go#L61-L73 There was a problem hiding this comment. What is the advantage of that? That means sources have to be able to deal with either or or there needs to be some sort of adapter to convert, both of which is very inconvenient. How does this improve the overall UX? There was a problem hiding this comment. Mostly for the ability to infer type, I’d say. Anytime that you specify There was a problem hiding this comment.
Both your and my approach leave the currently-exported interfaces in place, they have the same shape in both approaches. The difference in your approach is that because There was a problem hiding this comment. I’m trying to find a reason why I abandoned aliasing in the first place. Yes, it seems to be working, and having to specify a typedEvent[*Pod] is a minor inconvenience. My guess would be that the issue discussion lead to overcomplicating the solution. But I see a difference with using |
||
| // ObjectOld is the object from the event | ||
| ObjectOld T | ||
|
|
||
| // ObjectNew is the object from the event | ||
| ObjectNew T | ||
| } | ||
|
|
||
| // TypedDeleteEvent is an event where a Kubernetes object was deleted. TypedDeleteEvent should be generated | ||
| // by a source.Source and transformed into a reconcile.Request by an handler.TypedEventHandler. | ||
| type TypedDeleteEvent[T any] struct { | ||
| // Object is the object from the event | ||
| Object T | ||
|
|
||
| // DeleteStateUnknown is true if the Delete event was missed but we identified the object | ||
| // as having been deleted. | ||
| DeleteStateUnknown bool | ||
| } | ||
|
|
||
| // TypedGenericEvent is an event where the operation type is unknown (e.g. polling or event originating outside the cluster). | ||
| // TypedGenericEvent should be generated by a source.Source and transformed into a reconcile.Request by an | ||
| // handler.TypedEventHandler. | ||
| type TypedGenericEvent[T any] struct { | ||
| // Object is the object from the event | ||
| Object T | ||
| } | ||
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 think here a change suggested in #2685 will fit better. Projection might not work for different sources.
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.
This is only used to construct a source.Kind.
I disagree. Both what we did previously and what is done over there is IMHO hacky and will not work after the Source was started. Deferring the creation and creating it with the right args rather than manipulating it later on is IMHO much cleaner.