-
Notifications
You must be signed in to change notification settings - Fork 221
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
Custom API path /api/kernelspec #779
Comments
Hi @lucabem - its good to hear from you again. Kernel specs are not "user owned" but are "system owned". They merely contain the metadata used to start a kernel. For example, User A and User B can each start an instance of kernel 'foo' using the 'foo' kernelspec. You don't see the kernelspecs handlers explicitly defined in Enterprise Gateway because they're essentially inherited from Notebook. I think you're really talking about the Because there's already a Perhaps the MainKernelHandler's I would be happy to answer any questions you might have should you want to provide pull request for this. |
Maybe i have explained it bad @kevin-bates When we log in JupyterHub and we see home page, the system makes a REST call: What im trying to do is try to filter the response of that call to just let user select between kernel_1 and kernel_2 (if <user_logged> is user_1). We want to reach this call: Hope you have a nice day and thanks! |
In kernelspec.py (file from jupyter_client) there is a function called find_kernel_specs that could be usefull. I have seen that this function search form kernels on a directory. If we were able to pass the path from NFS join username we could filter by user. What i want to imitate is the nb_conda_kernels behaviour |
Hi Luis - thank you for the further explanation! Unfortunately, at this time, EG doesn't receive any kind of hint as to which user a given request is made on behalf of. An exception to that is the POST handler for /api/kernels (to start a kernel) will pass KERNEL_USERNAME in the request body - but only from NB2KG, Notebook/GatewayClient or a custom app hitting the EG REST api. If we were to do something similar for /api/kernelspecs, here's the approach I'd recommend...
By leveraging the authorized-users lists (and ensuring the query parameter is in that list and is NOT in the unauthorized-users list) we'd have some cohesion with kernel starts. If neither list exists or no parameter was provided, we let all kernelspecs through. I'd rather not rely on segments in a path hierarchy against which to apply a filter. Note: If you only need to prevent certain users from starting certain kernels, you can do this today. If user2 were to select a kernel that it's not authorized to use, user2 will receive a 403 error when attempting to start that kernel. However, we don't have a means of filtering the presentation of the kernels at this time. |
Thank you @kevin-bates - that souns great! Maybe it is as a bit greedy that estructure. I have read your post and i have noticed that if we change the name of our env, we could have them classified by username:
We could change the behaviour of MainKernelSpecHandler class. This method has this header So our function
|
Hi Luis, I would prefer we follow my previous suggestion and leverage the existing In addition, we cannot change the signature for the GET request. With what you have above, the pattern for obtaining the list of kernelspecs would go from... (r"/api/kernelspecs", MainKernelSpecHandler), to something like... kernel_user_regex = r"(?P<kernel_user>[\w%]+)"
...
(r"/api/kernelspecs/%s" % kernel_user_regex, MainKernelSpecHandler), I believe that's why we'd need a query parameter. We would then implement an internal method, something like def apply_user_filter(kernel_user, kernelspec_model):
# If user was specified continue, else skip everything and return the model
if kernel_user:
# User filter as been requested.
if kernelspec_model.metadata:
if kernelspec_model.metadata.process_proxy:
if kernelspec_model.metadata.process_proxy.config:
if kernelspec_model.metadata.process_proxy.config.unauthorized_users:
if kernel_user in kernelspec_model.metadata.process_proxy.config.unauthorized_users:
# This user is not authorized for this kernel...
return None
if kernelspec_model.metadata.process_proxy.config.authorized_users:
if kernel_user not in kernelspec_model.metadata.process_proxy.config.authorized_users:
# This user is not authorized for this kernel...
return None
return kernelspec_model The for loop in the handler would then look something like... for kernel_name, kernel_info in kspecs.items ():
try:
# Miramos si el kernel empieza con el parametro que queremos
if is_kernelspec_model (kernel_info):
d = kernel_info
else:
d = kernelspec_model (self, kernel_name, kernel_info['spec'], kernel_info['resource_dir'])
model = apply_user_filter(kernel_user, d)
if model:
specs[kernel_name] = model
except Exception:
self.log.error ("Failed to load kernel spec: '%s'", kernel_name, exc_info=True)
continue
Are you trying to imitate the behavior or are you actually using nb_conda_kernels? I believe nb_conda_kernels essentially creates a facade that there are kernel.json entries distributed across the conda environments, but there really isn't a physical kernel.json file in each. Instead, they hook the kernelspec manager class and add some thunking logic into the Another approach you might try is start an EG per user where each EG instance uses a different |
Hi @kevin-bates - Thanks for the solution! I have modified the class MainKernelSpecHandler as you said (with some changes). I have tested on PyCharm and now when we call api/kernelspecs/user/luis I recived only Kernels where test is on their authorized_users.
Take noticed that I have modified the URL |
Hi @lucabem - this sounds good. I was thinking that rather than register a different pattern, we'd instead offer the ability for an optional parameter, so if the caller wished to apply user filtering to the returned kernelspecs, they'd use: How are you picking off the username from |
Hi @kevin-bates! Instead of modifying the function get_kernel_spec, I have modified list_kernel_specs.
Now, the json response will contain only the kernels of 'USER' I have to test it using Dockerfile, JupyterHub and Kubernetes cluster (the filter works on local). |
Yes - use whatever is closest to where you want to be. I think I understand your approach a bit better now. If you're planning on contributing the changes (both in Notebook and EG) we'll need to talk first. I would really like to not have to introduce a new pattern for retrieving kernels. Also, rather than use Were you planning on contributing these changes? I think they'd be helpful, but we need to be sure they're a fit for all environments and not jeopardize existing functionality, etc. I'm happy to help guide things appropriately. |
Hi @kevin-bates!
It would be a pleasure, but maybe in 2 months. In this moment i dont have enough free time (im ending my degree) |
Sounds good. Good luck with your degree! |
Hi @kevin-bates! - I am facing some problems trying to spawn containers on K8s cluster. As you will remember, we have changed
You have said: MainKernelSpecHandler must also service /api/kernelspecs. What does it mean? I think my problem could be linked with this, because on JEG'logs i get an 404 error comming from /api/kernelspecs In addition, I have modified notebook to make a request to |
I'd have to see the various code changes you've made. I might also suggest you leave Are you able to push your changes to a fork that I can see? |
Hmmm.... yes, I have override the MainKernelHandler instead of creating a new class. Tomorrow i will fork it and upload changes. In addition, I have modified notebook/gateway to call /api/kernelspecs/user/{user} to use new JEG'path. |
Sounds good. Yeah, your change to gateway probably won't fly. This is one reason why I was thinking we'd extend the current RPC to include an optional query parameter. Then, notebook/gateways using the updated code would conditionally add the query parameter - probably based on the presence of KERNEL_USERNAME. If the target EG is an older version, I believe (not certain about this) that the parameter will be ignored - yielding all kernelspecs. While updated EG targets would detect the parameter and yield the filtered set of specs. Introducing an entirely new endpoint also introduces backward compatibility issues. |
Something i couldnt understand is the process when user select a new kernel. I know that JEG recived that request and it creates a pod into kubernetes cluster. But, what happens between notebook server request and JEG response? Maybe, we could just let this process to
|
Rigth now, my architecture is:
|
When
I suspect this is because Notebook (or the front end) is fetching the kernelspec directly and/or its resources (which are accessed by I haven't checked, but your What exception do you see when it "crashes"? |
Maybe i have explained it bad. When I select a new kernel, JEG'log gets me an 500 error POST and 404 error: /api/kernelspecs. Its not an exeception, just is not able to connect jupyter notebook with kernel |
No worries. I didn't expect an exception either. What do the EG and Notebook logs/consoles have in them relative to these issues? Please be specific as to which log contains what in your response. |
|
Finally it works. I have modified a bit MainKernelSpecHandler to:
|
Great - congratulations! Yeah, adding the primary endpoint back in probably helps. A few comments...
|
I have modified notebook package to fill the request with |
If you're setting up authorized-users lists then you'd have to add 'jovyan' to every user list - otherwise no kernel can be started. If The entire intention of kernel-pod names are Btw, |
Hi @lucabem - I'm sorry, I thought for sure I had replied to your question, but now realize I must have gotten distracted and never got back to sending my response! Testing is difficult! We have some unit tests and we have a Docker-based integration test that runs a barely viable YARN server. But you're absolutely correct, development/testing on k8s is tedious. Do you know of any tools/packages that might make that kind of thing easier? |
Neithe @kevin-bates , I just was building dockerfile and playing with logs. One question, Im building JEG on a EKS-cluster into Amazon Web Services. When we deploy JEG, on yaml we have the option of use to use as external-ip of the service. But in aws we dont have an master-node. Do you know how to solve it? I have use an IP of a worker node but if that node goes down, it will be unreachable |
Do you know how frequently worker nodes go down? Are they brought down after a certain amount of time or are you implying there was some failure that stopped the worker node? The "plan" is to implement HA/DR. We have some aspects of DR in place already provided the file in which kernel persistence is stored is on a shared file system such that when a new EG instance starts, it loads the persisted kernel information and reconnects to those kernels originally running on behalf of the previous EG. Where we really want to be is here where we can have multiple EG instances running and, should one EG instance go down, the kernel interaction would be sent to another EG instance that detects it doesn't know about that kernel and then loads its state from the persistence location. (We also want to implement other forms of kernel state persistence besides just files - like NoSQL databases, etc.) I know this "vaporware" isn't helpful right now, but that's the plan. |
I was thinking on change the jeg-service from NodePort to LoadBalacer, or using NGIX. |
The service type listed in the YAML or helm chart templates is a suggested value, but the idea is that things will vary with deployment and configuration. Feel free to adjust things to what works best for you. I see that we don't treat the If you deploy using the YAML file, it would be great to know what items did change and we could figure out how best to make the helm charts varied in this aspect. Thanks. |
Hi @kevin-bates! I have rigth now free time to add this changes (filter kernels shown by user query params). |
Hi @lucabem - thanks for checking back in - I hope you are well and your degree is finished - congratulations! Why don't we start with the EG-side changes first by posting a PR? From there we can work on solidifying the API. I'm going to re-open this issue since it has a rich history and we can associate it with the PR. |
@kevin-bates - Perfect! During this week i will try to sum up this issue and focus on what behaviour we are expecting. On question, how is your env for working? I mean, do you code on Unix or Windows? |
Sounds good!
My primary development is on a mac, but I spend a lot of time running EG on a small (3 node) RedHat Linux cluster that runs Hadoop YARN and Spark which I hit from a Notebook server running on my mac. I'm not sure how well (or even if) EG would run on a Windows server. I don't think we do things that would prevent that, but it's not a focus for us. |
Hi @kevin-bates - Thats the sum up of this issue. Current behavior
Kernelspecs handlers are inherited from notebook and they dont allow us to pass a query param to the call We have two options to modify
In both cases we need a method to filter kernels by user. On method Filter could be like that (it should be cleaned):
|
We'll also need to determine how a given client knows whether or not it should request user-filtered kernelspecs. I suspect this is going to have to be a configurable option. I think the default needs to be to return the model and only if kernel_user is not |
Yes - it should be on multi-user enviroments as JupyterHub, where we want to give only kernels of one user. In my opinion it could be a booleanr where TRUE will filter kernels and FALSE wont.
Yes, and if they are both, I would use the more restrictive option. |
Correct - deny always supersedes allow. Ok. That seems fine. What if we were to make the value of the "flag" be the name of the environment variable on which to filter kernelspecs and base Examples: I think we're going to find applications and platforms where the user identity varies and this would allow greater flexibility. |
It sounds great! With that kind of configurable option we avoid having to add modify the api path kernelspecs In this way, the call would still be the same unless the time we check if that parameter exists. |
Ok, summing up new advances we have reached that our new MainKernelSpecHandler will have a header such this: If we dont recive anything --> we return the whole kernelspec directory. I will try to start coding it this week. I think we will need to modify /api/kernelspecs to allow query params as /api/kernelspecs?user={user} |
Correct.
Check unauthorized first. If found, return None. Also, if not found in the authorized list, BUT the list is present (and non-empty), then None must also be returned. (An empty authorized_users list behaves as if one is not present in the first place.) Hmm - I just remembered that the {un}authorized_users lists can also be specified at a global level (to span kernelspecs). As a result, we need to consider combining the two similar to what we do when launching the kernel.
We need to ensure that |
Case A: kernel_user doesnt appear in any list. We return None Case B: kernel_user appears on unauthorized list. We return None Case C: kernel_user appears on authorized list. We return the kernel_spec Case D: we apply more restrictive list. We return None Case F: No lists appear. We give all kernel_specs |
Yes. Although to clarify Case A: kernel_user doesnt appear in any non-empty list. We return None Case F will be the default case since most folks don't leverage this functionality. |
Nice, I have coded that filter:
The exists function:
We could assign both paths to the same handler. We just need to check if kernel_user param is None or not.
|
The main problem will be the second call of /api/kernelspecs. I mean, when we open a notebook-server, it calls /api/kernelspecs to get available kernels. but when we select one kernel, it makes another call to /aoi/kernelspecs. im trying to know where and who is calling this second time. Could you help me with that? |
Hmm. All "requests" for kernelspecs in a Notebook server configured to hit a Gateway, should go through the Gateway classes that reside in Notebook, specifically, Btw, the Since we'll need Enterprise Gateway updated before Notebook, please go ahead and submit a PR to EG and we can work off that. Thanks. We need to also ensure that if user-filtering is configured on the gateway client (in Notebook), and its pointing at an older EG (that doesn't know about this), the requests still work or there's a clear indication that we can use to post a message that user-filtering is not available. |
I would close it. The pull request is nice! |
I have been stalking this issue for a while 👀 One minor comment I could make would be it would be nice to be able to filter kernels also based on the kernel.json path (/home/[user]/.local/share/jupyter/kernels/test_kernel/kernel.json). So that I could be automatically granted auth on my own kernels. |
@snu-ceyda - thank you for your interest in this. That's a clever way to trigger this - I forget what all is exposed. Regarding applying the filter to the path, I'd prefer we not "go there". This would require changes in libraries two-levels removed from EG (jupyter_client) and the rest of the ecosystem isn't really geared toward supporting the notion of multiple users simultaneously. |
Whata do you mean? In my free time I have written a KernelSpecManager modified to allow search into user home directory. Maybe it could help you |
Hi @kevin-bates:
I want to modify the behaviour of the API call /api/kernelspec. Now, when we start a notebook server via JupyterHub, this server makes a call to /api/kernelspec and the response is a list of kernels in the NFS server.
I want to filter that kernels by user who have created them. Instead of calling /api/kernelspec, it should be /api/kernelspec/{username}.
I have seen the swager.yaml but i dont find the python file of the API.
Thank you and have a nice day
The text was updated successfully, but these errors were encountered: