search

jupyter / kernel_gateway_demos

Demos associated with the kernel gateway incubator project
BSD 3-Clause "New" or "Revised" License
153 stars 76 forks source link

Graduate NB2KG #59

Closed lresende closed 6 years ago

lresende commented 6 years ago

We have been discussing throughout differents PRs that we should graduate NB2KG, and below are some of the suggested options:

Let's use this issue for discussing any other alternatives and/or suggestions in a centralized place

parente commented 6 years ago

We discussed moving the nb2kg code to https://github.com/jupyter-incubator/nb2kg during the Jupyter dev call today. No one there had any objections. I can take this ticket to move the code sometime this week or next.

parente commented 6 years ago

As for merging it into notebook itself, two possible approaches come to mind:

  1. Send the PR to notebook with a hefty description as to why, what it's enabling, and have a discussion there with code in hand.
  2. Get people more familiar and comfortable with nb2kg as its own incubator project, state from the beginning the goal is to ultimately get it into notebook, work out any kinks there, and then send the PR to notebook down the line.

I don't have a read on people's thoughts on nb2kg functionality directly in notebook at the moment to know which is better.

kevin-bates commented 6 years ago

That would be great - thanks @parente.

Although I'd love to have nb2kg embedded in Notebook - as @lresende mentions - I believe it would be viewed as opening the floodgates for a number of other server extensions (i.e., where do you stop?) even though it would be fairly trivial and lightweight to bolt it into NB. I'm curious if that avenue was discussed today (sorry, I guess our updates crossed - leaving mine as is). Yes, I suppose we can take approach 2 for now.

Once moved, I think we should publish an update to pypi (and conda forge?).

parente commented 6 years ago

I found jupyter/notebook#3006 about adding configuration overrides to support custom handlers in notebook. I am delaying creating the jupyter-incubator/nb2kg repo to see how that PR shakes out. If you'd like the incubator project created ASAP however, give a shout.

kevin-bates commented 6 years ago

Thanks Peter - sorry for the delayed response.

Since the jupyter/notebook#3006 PR falls short on actually merging NB2KG into the Notebook repo, I'm inclined to move forward with NB2KG's promotion to an incubator for the following reasons.

  1. Merging into Notebook will require further, and perhaps extensive, discussion and I think we need to publish a 0.0.2 release sooner.
  2. There may be some other items that come up that warrant changes that would be good to get in before the merge since we'd want NB2KG to be virtually complete as post-merge changes will require more time to get published.

One area that I'd like to see improved is richer error messaging back to the client (from the gateway). While the gateways (kernel and enterprise) raise detailed exceptions (that are logged on the gateway server), only the error code seems to be propagated. I'm unfamiliar with HTTP error messaging and haven't actually looked into it, so I'm uncertain there's more that can be done, but I think improvement in that area could go a long way to helping our gateway clients.

I'd love to hear others' opinions.

parente commented 6 years ago

I went ahead and created https://github.com/jupyter-incubator/nb2kg. If it turns out the extension is not needed one day, removing the repo can be a form of graduation.

I used the git filter-branch command to extract the nb2kg subfolder with its full history from this project and pushed it as the master branch in the new one. I invited all of the collaborators from jupyter-incubator/enterprise_gateway to then new project with equivalent permissions. I also added the original collaborators from the kernel_gateway_demos project. A nb2kg package was never pushed to pypi, so there's no permissions to configure there.

I'm going to submit a PR here momentarily that removes the nb2kg code from this repo and leaves a simple README in its place pointing to the new location.

kevin-bates commented 6 years ago

Awesome, thank you @parente! I'll update README.md since I'd like to add reference to enterprise gateway anyway.