Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Informer gen keyfuncs #2

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Informer gen keyfuncs #2

wants to merge 4 commits into from

Conversation

fabianvf
Copy link
Owner

@fabianvf fabianvf commented Jun 2, 2022

No description provided.

@fabianvf fabianvf force-pushed the informer-gen-keyfuncs branch 7 times, most recently from e148e4b to 9691028 Compare June 2, 2022 17:43
Use variadic option function instead of struct

handle defaulting of sharedIndexInformer.index

add godocs
@@ -83,6 +85,8 @@ type NewInformerFunc func({{.clientSetPackage|raw}}, {{.timeDuration|raw}}) cach
type SharedInformerFactory interface {
Start(stopCh <-chan struct{})
InformerFor(obj {{.runtimeObject|raw}}, newFunc NewInformerFunc) {{.cacheSharedIndexInformer|raw}}
ExtraIndexers() {{.cacheIndexers|raw}}
KeyFunction() {{.cacheKeyFunc|raw}}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One comment from Jordan that I didn't mention before (sorry!) was trying to avoid needing to do this. We should see if we can adjust the default informer functions in some way so this isn't needed.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

think I addressed it. I added variadic options to the NewInformerFunc type. I was unable to find a case of someone defining their own implementation of that function so as long as we don't take out the redundant resyncPeriod all callsites should remain unbroken. I could also create a parallel implementation with a NewInformerFuncWithOptions and InformerForWithOptions but it seemed a little heavy considering most of that is internal generated code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants