-
Notifications
You must be signed in to change notification settings - Fork 390
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
Query parameter naming scheme? #712
Comments
I think at least recommending that extensions use |
There's also the option of passing JSON as a 'resourceRequests' parameter. |
Kubernetes resources are pretty much key value pairs of string -> integer, with some special casing. If you read https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/, you'll see that:
I think we should do something similar, and define in the 'binderhub runtime parameters spec' a key-value pair mapping for resources, with some well known keys as special case. I like that the values are integers with a small base unit (byte for memory or microcpu for CPU) - this keeps us out of floating point precision issues, and translates well to how most resource managers see these resources. These can then be encoded in the URL in many ways. My favorite way is to just repeat the parameter, and use get_query_arguments to get it as a list. So you'd end up with:
For the UI, operators can define commonly used combinations of requests into human readable names that are shown in the UI. This makes it user friendly, while making the launch links themselves be consistent across binderhub installations and time (as ideas of what is 'small-cpu' change). Other things we should define are:
We should also define a way for repos to 'suggest' runtime parameters (including resources) in a file in the repo, but that's probably a different issue. Thanks to @craig-willis for bringing that up! |
Thanks @yuvipanda - this all makes sense.
In additional to defaults for specific resources, I think we should also have maximum requests.
I don't think we want binderhub to know very much about the resources available for user sessions. Instead, we should rely on kubernetes to let us know if specific resources are not available. We should endeavor to help users understand what "Unschedulable" means |
I think there are a couple of questions about limits and what to do when they are exceeded in different ways:
|
+1 on (1) and (2).
I think binderhub should explicitly fail in this case. I think it would be confusing to request 4GB of ram and then have an application die from a memory error at 2GB. |
Was recently looking into facilitating getting users running binderhub with a GPU and ran into this issue. I think this functionality would be amazing! A bit more background on a specific use-case: We are already running a binderhub, which has a single 'user' nodegroup and the resource requests are fixed. I'd like to add a second 'user-gpu' nodegroup that pods get scheduled on only if a GPU is needed as we do in jupyterhub. Current options seem to be 1) run an entirely separate binderhub or 2) refer people to Google Colab :( For JupyterHub this is simply a matter of of adding some key-value pairs under kubespawner override:
If I understand the flow correctly I think a workaround can be implemented by adding some custom code to the helm chart spawner configuration here by parsing the URL and adding the right kubespawner_override settings?
|
Should we agree a "standard" way of extending the BinderHub URL schema?
The particular use case for me is that I'd like to be able to pass extra information to the spawner via the launch URL. For example which of a set of resource constraints to use or what set of extra volumes to mount on launch.
Right now thinking something like:
with the spawner knowing how to convert "lots-more", "small-cpu", etc into a set of actual constraints /volume specs.
Right now I can pick essentially any name but I was wondering if we should agree that (say) the
x-
prefix will be reserved for people to do "what they want" and BinderHub itself will only use none prefixed query parameters.The text was updated successfully, but these errors were encountered: