Embed NB2KG into Notebook server

[kevin-bates]

This change alleviates a significant pain point for consumers of Jupyter Kernel and Enterprise Gateway projects by embedding the few classes defined in the NB2KG server extension directly into the Notebook server. What took multiple (and error-prone) steps previously can now be done via a single option - leading to more uses of the Jupyter ecosystem across large installations.

• All code resides in a separate gateway directory that is enabled via a new configuration option --gateway-url.

• Renamed classes from those used in standard NB2KG code so that Notebook servers using the existing NB2KG extension will still work.

• Added test_gateway.py to exercise overridden methods. It does this by mocking the call that issues requests to the gateway server.

• Updated the Running a notebook server topic to include a description of this feature.

[kevin-bates]

Updating to [WIP] as there's one thing I want to check out. I'll also rebase with recent merges.

[kevin-bates]

I was hoping, at a minimum, to get this into jupyter_server but was told previously that PRs should be posted to Notebook for now. I understand this may seem like a major change, but it's really just a means of proxying the kernel management to a different server. It dramatically simplifies deployment of the Jupyter Gateway projects and should increase the adoption of Jupyter projects at installations that require separation of notebooks from compute resources.

[minrk]

I'd like to avoid any environment-only configuration in this repo. All configuration should be done through traitlets. Maybe this warrants a KernelGateway configurable class to put these on? The defaults can still come from these environment variables if need be.

[kevin-bates]

I see - that makes sense. Is that how parameters like --<className>.<parameter> work? Some of these are referenced in handlers and managers. Would this be accomplished by creating a Configurable subclass (e.g., Gateway) which would then be referenced via multiple inheritance in the handler/manager classes?

[minrk]

Exactly right. It would look a bit like:

from traitlets import Unicode, default
from traitlets.config import LoggingConfigurable


class Gateway(LoggingConfigurable):
    client_key = Unicode(
        config=True,
        help="""gateway client key for...""",
   )
   @default('client_key')
    def _default_client_key(self):
        return os.getenv('JUPYTER_GATEWAY_CLIENT_KEY', '')

and then somewhere in NotebookApp:

self.gateway = Gateway(parent=self)

which would then be referenced via multiple inheritance in the handler/manager classes?

I'd use a "has a" relationship, because multiple inheritance of Configurables and Handlers doesn't seem to play very nicely together. So the handlers would access self.gateway.client_key etc.

[kevin-bates]

Beautiful - thank you for the example. I've moved the PR into WIP until I've had a chance to implement these changes.

[minrk]

Thanks for the PR! I've been thinking about this one and trying to decide the right path forward.

On one level, promoting this functionality is stating that KG functionality is stable and general enough to be promoted to distributing to all users (exciting!), and won't be making breaking changes any time soon. Does this entirely deprecate the gateway repos, or is there still code that resides there?

On another level, at least as I interpret the PR description, this seems to be declaring that the extension APIs for Jupyter are just too unstable and brittle to be able to write and maintain extensions like KG. That seems like a problem in itself and maybe one we should look at, and this approach is avoiding the problem for one specific extension while leaving all others to deal with the problems without fixing them. What do you think, @kevin-bates? Can you describe the problems that are solved by pinning this to the notebook version vs allowing it to live in an extension?

[lresende]

Thanks for the PR! I've been thinking about this one and trying to decide the right path forward.

On one level, promoting this functionality is stating that KG functionality is stable and general enough to be promoted to distributing to all users (exciting!), and won't be making breaking changes any time soon. Does this entirely deprecate the gateway repos, or is there still code that resides there?

I would put this in a slightly different way, this PR bring to Jupyter native support to connect to a remote gateway (kernel gateway or enterprise gateway) server. In this scenario, you still need a Gateway in the picture thus the gateway repositories are still valid/required and NB2KG repository becomes deprecated for new versions of Jupyter.

On another level, at least as I interpret the PR description, this seems to be declaring that the extension APIs for Jupyter are just too unstable and brittle to be able to write and maintain extensions like KG. That seems like a problem in itself and maybe one we should look at, and this approach is avoiding the problem for one specific extension while leaving all others to deal with the problems without fixing them. What do you think, @kevin-bates? Can you describe the problems that are solved by pinning this to the notebook version vs allowing it to live in an extension?

As we see the adoption of gateway functionality growing, we also see growing user confusion due to missing or misconfigured NB2KG and thus not being able to connect Jupyter to Gateway properly.

My view on this is is that, by bringing this support directly into Jupyter, we will be enhancing the user experience and eliminating a significant pain point to Jupyter users that want to connect to a Gateway server.

@kevin-bates any other details you might want to add here ?

[kevin-bates]

@minrk - thank you for taking a look at this PR and you comment/question. @lresende is correct - this PR eliminates the need for the NB2KG server extension, yet either Gateway instance (running at --gateway-url) is still required.

Where this will also be of great benefit is for enabling JupyterHub to launch gateway-destined notebooks - enabling those kernels to be launched against managed clusters. By significantly reducing the barrier to entry gateway we increase user options without having to understand what NB2KG is or that it even exists.

Regarding the usability of server extensions, I think NB2KG tends to be different from other extensions in that it requires at least one configurable item itself - the url at which the destination gateway resides - in addition to the class mapping overrides for the three *Manager classes that proxy the kernel management. These two aspects tend to trip up users, and that's assuming they've pip-installed the extension and it's been registered/enabled successfully. So, although I'm not familiar with other server extensions, I'm guessing the "configuration" required for NB2KG is unique. This may be more due to the fact that this is truly performing server-side operations at lower than normal levels.

[minrk]

Thanks for the background! I think Extensions being able to register various Manager classes has come up before, and might be something we could explore (i.e. users would only enable the extension, and then the extension would be responsible for instantiating the managers, which I believe is not possible now). We could explore that in general, but I think it is sensible to bring Gateway functionality in here, as long as it is sufficiently stable.

[kevin-bates]

@minrk - thank you for the suggestion of moving the env variables to config options! I've created a SingletonConfigurable and this has led to some nice cleanup.

I've kept the commits separate, but can squash (and rebase with master) if you like.

Thanks!

[minrk]

There appears to be a lot of duplication of handlers here. Ideally, there would be ~no duplication of Handlers, only the Manager implementations, but I think the channels URL at least needs special handling. It doesn't appear to me that the kernel listing, action, or kernel spec handlers need to be replaced, though.

[kevin-bates]

Good point - that makes sense. Taking a quick look, it appears there are some methods that have been added to Notebook since NB2KG was first implemented. I should be able to create appropriate methods (that hit the gateway or equivalent). I'll move this back to WIP again.

Question... How would you recommend the handler load for path r"/api/kernels/%s/channels" % _kernel_id_regex be handled, since use of the gateway will still require the NB kernel handlers - where its own channels handler is also registered? Does the order of the handlers list matter and, if so, is it a first match wins behavior? If handlers are unordered, then I'll need to conditionally perform replacement within the handlers list in init_handlers() to get the appropriate channels handler registered.

[kevin-bates]

@minrk - ping - is there anything else I should do?

[lresende]

@minrk Do you have any other issues/suggestions based on your review? Or is this ready to be merged?

[lordcirth]

@minrk Is there anything blocking this from being merged? Thanks.

[minrk]

Thanks for your patience on this one, I think it's good to go now.