-
-
Notifications
You must be signed in to change notification settings - Fork 151
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
Integrate with jupyterhub launcher #189
Conversation
cc @jacobtomlinson ? |
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.
Very keen to support this use case.
I saw you included this in your Arm PR and I took it out again because we were no longer able to just run the container and have Jupyter Lab start. I wasn't sure what removing start.sh jupyter lab
was trying to achieve there. But I see now.
We need to ensure that a simple command like this works.
$ docker run --rm -it daskdev/dask-notebook:latest
To achieve this I think we need to update the start command in the Dockerfile
and possibly check the order of the exec args.
# Execute the jupyterlab as specified. | ||
exec start.sh jupyter lab ${JUPYTERLAB_ARGS} | ||
# Execute the jupyterlab as specified from either JUPYTERLAB_ARGS or command line args. | ||
exec ${JUPYTERLAB_ARGS} "$@" |
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 think this should be
exec ${JUPYTERLAB_ARGS} "$@" | |
exec "$@ ${JUPYTERLAB_ARGS}" |
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 also wonder if we should rename it to EXTRA_JUPYTERLAB_ARGS
to make it clear what it is. This isn't really and either or situation, the command should always be passed via the $@
.
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.
extra works too :)
Ah so I just modified the docker compose so it would pass arguments, but yeah I can change this have a default no args too 👍 |
This PR is pretty stale now, sorry we didn't manage to get it merged. I'm going to close it out now. |
This allows the container to be used from the zero-to-jupyterhub launcher (aka kubespawner).