wilfred-s commented on code in PR #844:
URL: https://github.com/apache/yunikorn-k8shim/pull/844#discussion_r1609379727
##########
pkg/client/clients.go:
##########
@@ -27,13 +27,19 @@ import (
coreInformerV1 "k8s.io/client-go/informers/core/v1"
schedulingInformerV1 "k8s.io/client-go/informers/scheduling/v1"
storageInformerV1 "k8s.io/client-go/informers/storage/v1"
+ "k8s.io/client-go/tools/cache"
+ "k8s.io/klog/v2"
"k8s.io/kubernetes/pkg/scheduler/framework/plugins/volumebinding"
"github.com/apache/yunikorn-k8shim/pkg/conf"
"github.com/apache/yunikorn-k8shim/pkg/log"
"github.com/apache/yunikorn-scheduler-interface/lib/go/api"
)
+type informer interface {
Review Comment:
I looked at using something like slices for storing all the different
informers. Due to the differences of the listers between the informers that
becomes a slice of `interface{}` That then triggers type casting and all kinds
of other handling and checks which is worse than having the separate variables
and maintaining the two sets. Just storing the `Informer()` and not the
`Lister()` for the informers does not make sense for me. The lister is normally
accessed outside of startup the informer is never accessed as it just runs...
Any informer defined should be started and stopped. To not regress we could
have a simple unit test that checks that all informers defined in the type are
also returned in the `getInformers()` call. If someone adds a new informer we
can then ensure that they are updated in both places
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
