Improve UX for using client-go generated Informers #181
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pwittrock 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
cncf-cla: yes
k8s-ci-robot
added
approved
k8s-ci-robot
added
size/L
| @@ -135,6 +135,14 @@ func (r RunnableFunc) Start(s <-chan struct{}) error { | |||
| return r(s) | |||
| } | |||
|
|
|||
| // StartAdapter wraps a Start function to make it implement Runnable | |||
| func StartAdapter(s func(<-chan struct{})) Runnable { | |||
There was a problem hiding this comment.
To a new user it will read as ---> This starts some sort of an adapter. Other names that comes to mind are: MakeRunnable, StartFnAdapter.
| s(c) | ||
| return nil | ||
| }) | ||
| } |
There was a problem hiding this comment.
I would like manager pkg to export just the essentials functions/methods. Functions like these, which are syntax sugar, should belong to some sister utils pkg like we controllerutils for controller helpers.
Backward compatibility contracts with core methods/functions are going to be more stricter than these utils/helpers pkgs. WDYT ?
| // Setup Watch using the client-go generated Informer | ||
| if err := ctrl.Watch( | ||
| &source.Informer{InformerProvider: generatedInformers.Core().V1().Services()}, | ||
| &handler.EnqueueRequestForObject{}, |
There was a problem hiding this comment.
If I am reading correctly, there is an order requirement here. Informer source should only start after the informer Runnable has started and sync'ed ? Not sure how that is enforced. Will look into the code. I may be wrong also about the ordering requirement.
| Informer toolscache.SharedIndexInformer | ||
|
|
||
| // InformerProvider provides a generated client-go Informer. Mutually exclusive with Informer. | ||
| InformerProvider InformerProvider |
There was a problem hiding this comment.
Informer term is being overused and source.Informer and client-go.Informer are getting mixed up in my head and causing confusion. Don't have a better suggestions.
Add test for memcached-api-server project
No description provided.