pbacsko commented on code in PR #844:
URL: https://github.com/apache/yunikorn-k8shim/pull/844#discussion_r1635384659
##########
pkg/client/apifactory.go:
##########
@@ -64,7 +44,7 @@ type APIProvider interface {
// resource handlers defines add/update/delete operations in response to the
corresponding resources updates.
// The associated the type field points the handler functions to the correct
receiver.
type ResourceEventHandlers struct {
- Type
+ InformerType
Review Comment:
Was this changed to avoid confusion with `type`?
##########
pkg/client/clients.go:
##########
@@ -63,42 +92,96 @@ type Clients struct {
VolumeBinder volumebinding.SchedulerVolumeBinder
}
+func NewClients(scheduler api.SchedulerAPI, informerFactory
informers.SharedInformerFactory, configs *conf.SchedulerConf, kubeClient
KubeClient) *Clients {
+ // init informers
+ // volume informers are also used to get the Listers for the predicates
+ hasInformers := []hasInformer{}
+
+ podInformer := save(informerFactory.Core().V1().Pods(), &hasInformers)
+ nodeInformer := save(informerFactory.Core().V1().Nodes(), &hasInformers)
+ configMapInformer := save(informerFactory.Core().V1().ConfigMaps(),
&hasInformers)
+ storageInformer :=
save(informerFactory.Storage().V1().StorageClasses(), &hasInformers)
+ pvInformer := save(informerFactory.Core().V1().PersistentVolumes(),
&hasInformers)
+ pvcInformer :=
save(informerFactory.Core().V1().PersistentVolumeClaims(), &hasInformers)
+ priorityClassInformer :=
save(informerFactory.Scheduling().V1().PriorityClasses(), &hasInformers)
+ namespaceInformer := save(informerFactory.Core().V1().Namespaces(),
&hasInformers)
+ csiNodeInformer := save(informerFactory.Storage().V1().CSINodes(),
&hasInformers)
+ csiDriversInformer := save(informerFactory.Storage().V1().CSIDrivers(),
&hasInformers)
+ csiStorageCapacityInformer :=
save(informerFactory.Storage().V1().CSIStorageCapacities(), &hasInformers)
Review Comment:
Honestly, I don't know. We do save some lines by introducing this
intermediate type, at the same, readability is not the best. In fact, I had to
download and apply the patch locally to understand what's happening here.
I'm not the fan of this.
For example, even though this is more LOC, it's just much simpler:
```
var sharedInformers []cache.SharedIndexInformer
podInformer := informerFactory.Core().V1().Pods()
sharedInformers = append(sharedInformers, podInformer.Informer())
nodeInformer := informerFactory.Core().V1().Nodes()
sharedInformers = append(sharedInformers, nodeInformer.Informer())
...
```
Then `sharedInformers` can be passed as a slice to `Client` after it's done.
This way, we also don't need to copy the slice every time we call
`getInformers()`, another thing which I don't really like.
--
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]
