-
Notifications
You must be signed in to change notification settings - Fork 303
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
share kubernetes client instances #128
Conversation
we should hold off on this until after 0.6 of z2jh. |
yes, I was planning to cut the release and finish z2jh before merging this |
avoids creating a kubernetes client for each Spawner, which creates a bunch of threads in kubernetes 4.0. uses weakref to avoid breaking garbage collection
962e9e3
to
65757f5
Compare
@minrk is this good to go or still WIP? |
kubespawner/clients.py
Outdated
cache_key = (ClientType, args, kwarg_key) | ||
client = None | ||
if cache_key in _client_cache: | ||
client = _client_cache[cache_key]() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the trailing ()
? I think _client_cache
contains instances already so this would end up calling __call__
on that instance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_client_cache
contains weak references to instances, not the instances themselves. This allows them to be garbage collected. The ref must be called in order to return the instance. If it has been garbage collected in the meantime, resolving the weakref will return None.
I've added a comment to clarify what's happening here.
I've
Forgot to take off WIP that was meant to block it until shipping v0.6. This should be all set now. |
Merging as travis is happy. Thanks for explaining the |
avoids creating a kubernetes client for each Spawner, which creates a bunch of threads in kubernetes 4.0.
uses weakref to avoid breaking garbage collection
closes #126