-
Notifications
You must be signed in to change notification settings - Fork 5k
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
Update gateway support with recent changes #4431
Conversation
Convey notebook working directory to the gateway (nb2kg pr-21) Support retrieval of kernelspec resources from the gateway (nb2kg pr-23)
b6e851e
to
46bcf78
Compare
raise | ||
else: | ||
kernel_spec_resource = response.body | ||
raise gen.Return(kernel_spec_resource) |
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.
FWIW, since we require Python 3, we can use return kernel_spec_resource
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.
Ok - thanks. Since this is one of many gen.Returns, I think it makes sense to change these whenever we "make the sweep" to move to async/await - if that's okay with you.
@@ -91,6 +91,16 @@ def _kernels_endpoint_default(self): | |||
def _kernelspecs_endpoint_default(self): | |||
return os.environ.get(self.kernelspecs_endpoint_env, self.kernelspecs_endpoint_default_value) | |||
|
|||
kernelspecs_resource_endpoint_default_value = '/kernelspecs' |
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.
Does every endpoint need to be separately configurable? It seems to me like a single base path should be everything that's needed.
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.
I believe these are configurable (in NB2KG is done via just envs) for cases when the gateway server (JKG or JEG) is behind a reverse proxy. This should be the last of them. (yeah, famous last words, I know. 😄)
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.
ok, let's switch this over to a single configurable prefix before adding any more after this one.
Convey notebook working directory to the gateway (nb2kg pr-21)
Support retrieval of kernelspec resources from the gateway (nb2kg pr-23)