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

Switch to using kubernetes_asyncio #421

Closed
yuvipanda opened this issue Jul 27, 2020 · 0 comments
Closed

Switch to using kubernetes_asyncio #421

yuvipanda opened this issue Jul 27, 2020 · 0 comments

Comments

@yuvipanda
Copy link
Collaborator

yuvipanda commented Jul 27, 2020

Proposed change

We currently use the synchronous kubernetes client. Since kubespawner is tornado based, we need to make it work asynchronously. So we use threadpools. However, threads are hard! Async is hard! Mixing those two, as much as we do here, is hard! Forming a complete mental model of how the different threadpools in the application interact with each other to affect throughput is extremely hard - see #419 for example.

I suggest we move the k8s calls to kubernetes_asyncio. dask-kubernetes started with this from the beginning, and it's been a great experience. It's generated in exactly the same way the upstream synchronous client is generated, so maintenance & new features have been good. More context in [https://github.com/kubernetes-client/python/issues/323](this issue)

Alternative options

  1. Keep using the current library, but develop a better understanding of threadpool interactions
  2. Build on top of kube-rs. The big advantage here is that it's got fairly complete implementations of higher level kubernetes primitives like watchers & controllers. We implement them ourselves in python, but I'm not sure how thorough they are. However, as a downside, it's in Rust and will need effort to integrate into our codebase.

Who would use this feature?

  1. Developers who are working with the codebase. Threading is hard.
  2. Users who want higher throughput.

(Optional): Suggest a solution

I think we have enough tests here and in z2jh to be able to do this. The syntax is pretty compatible, so it would be a matter of going through the code base and making changes.

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

No branches or pull requests

2 participants