Initial draft for SharedInformer Support #1728
Conversation
rohanKanojia
force-pushed
the
pr/informers
branch
from
9507b3a
to
675fc5a
Compare
| @@ -316,4 +319,9 @@ public SubjectAccessReviewDSL subjectAccessReviewAuth() { | |||
| return new SubjectAccessReviewDSLImpl(httpClient, getConfiguration()); | |||
| } | |||
|
|
|||
| @Override | |||
| public SharedInformerFactory informers() { | |||
| return new SharedInformerFactory(Executors.newCachedThreadPool(), httpClient, getConfiguration()); | |||
There was a problem hiding this comment.
A new cached pool being created on every method invocation may not be a good idea, perhaps pass the ExecutorService as a parameter? Maybe use ForkJoinPool.commonPool() if a parameterless method is needed.
| import java.util.function.Supplier; | ||
|
|
||
| /** | ||
| * Controller is a generic controller framework. |
There was a problem hiding this comment.
This looks really complex. A generic framework lying in an utils package for a specific feature makes me wonder if this shouldn't belong to another package or library
There was a problem hiding this comment.
oh, okay. I would move it to a dedicated package while updating this PR
There was a problem hiding this comment.
It looks like it was copied from https://github.com/kubernetes-client/java/blob/master/util/src/main/java/io/kubernetes/client/informer/cache/Controller.java? I wonder if we could implement this feature without these classes, otherwise it would be better to add a reference saying that it was taken from this project
There was a problem hiding this comment.
I'm not sure we can do that. Okay, I'll add reference.
There was a problem hiding this comment.
This all has been ported from client-go codebase(https://github.com/kubernetes/client-go/blob/master/tools/cache/controller.go). I looked into official client's code and tried to fit those in our client since we need informers too.
rohanKanojia
force-pushed
the
pr/informers
branch
from
675fc5a
to
c8b063f
Compare
|
I tried writing a simple operator using this support and I was able to write it: https://github.com/rohanKanojia/podsetoperatorinjava |
kubernetes-client/src/main/java/io/fabric8/kubernetes/client/DefaultKubernetesClient.java
Show resolved
Hide resolved
rohanKanojia
force-pushed
the
pr/informers
branch
2 times, most recently
from
f0f2ec2
to
6fe7588
Compare
rohanKanojia
force-pushed
the
pr/informers
branch
from
6fe7588
to
3b24072
Compare
|
[merge] |
Fix #1384