Some fixes for Jupyter Notebook 7 #1944
Conversation
| @@ -38,7 +38,8 @@ WORKDIR /tmp | |||
| RUN mamba install --yes \ | |||
| 'notebook' \ | |||
| 'jupyterhub' \ | |||
| 'jupyterlab' && \ | |||
| 'jupyterlab' \ | |||
| 'nbclassic' && \ | |||
There was a problem hiding this comment.
It seems that we need to install this explicitly, and nbclassic is kept to make migration easier. So, our images should also support nbclassic.
https://jupyter-notebook.readthedocs.io/en/stable/migrating/multiple-interfaces.html#nbclassic-and-notebook-7
There was a problem hiding this comment.
That's right. The classic notebook UI is then available under the /nbclassic endpoint.
base-notebook/start-notebook.sh
Outdated
| @@ -19,4 +19,4 @@ if [[ "${RESTARTABLE}" == "yes" ]]; then | |||
| fi | |||
|
|
|||
| # shellcheck disable=SC1091,SC2086 | |||
| exec /usr/local/bin/start.sh ${wrapper} jupyter ${DOCKER_STACKS_JUPYTER_CMD} ${NOTEBOOK_ARGS} "$@" | |||
| exec /usr/local/bin/start.sh ${wrapper} jupyter ${DOCKER_STACKS_JUPYTER_CMD} --port ${JUPYTER_PORT} ${NOTEBOOK_ARGS} "$@" | |||
There was a problem hiding this comment.
JUPYTER_PORT env variable is no longer considered as a port for jupyter applications, so we pass this explicitly - I think the easiest way to do this is to add this line to both start-notebook.sh and start.sh.
We, on the other hand, can and should keep the variable name JUPYTER_PORT for users of our docker image.
This way we won't break them (unless they call start.sh manually, but I suppose it's a rare case).
There was a problem hiding this comment.
Normally this should still be supported I think?
There was a problem hiding this comment.
Since it's part of Jupyter Server now: https://github.com/jupyter-server/jupyter_server/blob/c1d689e05169ba47e630ccaa39cebb4f847b3562/jupyter_server/serverapp.py#L950
There was a problem hiding this comment.
Thanks, I will try to revert this part.
|
@jtpio may I ask you to take a look here - I think you know quite well if this is a reasonable change after Jupyter Notebook 7 release. |
| @@ -64,7 +64,7 @@ RUN mamba install --yes \ | |||
| # Install facets which does not have a pip or conda package at the moment | |||
| WORKDIR /tmp | |||
| RUN git clone https://github.com/PAIR-code/facets.git && \ | |||
| jupyter nbextension install facets/facets-dist/ --sys-prefix && \ | |||
| jupyter nbclassic-extension install facets/facets-dist/ --sys-prefix && \ | |||
There was a problem hiding this comment.
It seems that the command name has changed.
https://discourse.jupyter.org/t/notebookv7-nbclassic-how-do-you-enable-disable-classic-extensions-from-the-command-line/16344
There was a problem hiding this comment.
For reference there is also a note in the docs: https://nbclassic.readthedocs.io/en/latest/nbclassic_dev_faq.html#noteworthy-updates-in-nbclassic
|
Thanks @mathbunnyru for looking into this! I left a couple of comments, looking good otherwise 👍 |
|
@mathbunnyru @jtpio Since this pull request got merged (b8d617d), zero-to-jupyterhub-k8s always launches the singleuser jupyter environment in the classic notebook tree view, instead of the JupyterLab mode. I tried many different configuration options to force it otherwise, but I did not manage to change it. If I go back to the previous docker-stacks image (141d72d, there are only documentation related changes between the merged pull request and that version), it works as expected and I got the |
|
Thanks @kondi for reporting. I wonder if this might be related to jupyter/notebook#6954. |
|
@kondi @jtpio I also think there is a bug in the JupyterHub which won't allow to choose Jupyter Notebook properly by setting |
* Some fixes for Jupyter Notebook 7 * Fix jupyter nbextension * Remove explicit port setting
* Some fixes for Jupyter Notebook 7 * Fix jupyter nbextension * Remove explicit port setting
Describe your changes
Issue ticket if applicable
Checklist (especially for first-time contributors)