Skip to content
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

Install Jupyter kernel discovery and use nbgitpuller for examples #252

Closed

Conversation

jacobtomlinson
Copy link
Member

I'm working on using the RAPIDS Docker images with Jupyter Hub.

Two small tools that would be really helpful are nb_conda_kernels for conda environment discovery in Jupyter and nbgitpuller for downloading example notebooks.

We already download the example notebooks in the RAPIDS runtime image. But nbgitpuller is a small utility made by the Jupyter folks to automate this, and specifically automate updating these examples. So when we launch the docker image with JupyterHub we can easily update the examples at runtime.

@jacobtomlinson jacobtomlinson requested a review from a team as a code owner January 27, 2021 12:40
Copy link
Member

@quasiben quasiben left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to using nbgitpuller I think this also what nb_conda_kernels use as well

Copy link
Member

@ajschmidt8 ajschmidt8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couple questsions/comments below

@@ -7,7 +7,7 @@ RUN gpuci_conda_retry install -y -n rapids \
"rapids-notebook-env=${RAPIDS_VER}*"

{# Install jupyter lab stuff #}
RUN gpuci_conda_retry install -y -n rapids jupyterlab-nvdashboard
RUN gpuci_conda_retry install -y -n rapids jupyterlab-nvdashboard nb_conda_kernels nbgitpuller
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these seem like cool additions, but i have some questions/comments

  1. The addition of nb_conda_kernels should be fine. However, you've uncovered a bit of tech debt here on our end. We try to keep all the notebook dependencies in the rapids-notebook-env meta-package in the integration repo. So ideally jupyterlab-nvdashboard and nb_conda_kernels would both be added there.

  2. Our notebooks repo consist mostly of a bunch of git-submodules. When I tested the new gitpuller command locally, it didn't seem to also clone in the sub-modules (see screenshot below). After exploring the usage section in gitpuller --help, I didn't see anything that mentioned submodules so I'm not sure if it's compatible. Also, the notebooks repo (like other RAPIDS repos) has a default branch that changes every release, so I'm not sure how this would affect the functionality of gitpuller. Any thoughts?

image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Ah ok that's good to know. Should I open a PR over there?
  2. Interesting that it doesn't grab the submodules. Looks like there is an open PR to resolve Update submodules and do recursive clone jupyterhub/nbgitpuller#123. Also my intention on using the ${BUILD_BRANCH} env var was that the correct branch would be pulled.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Ah ok that's good to know. Should I open a PR over there?
  2. Interesting that it doesn't grab the submodules. Looks like there is an open PR to resolve jupyterhub/nbgitpuller#123. Also my intention on using the ${BUILD_BRANCH} env var was that the correct branch would be pulled.
  1. Yes, if you don't mind that would be great. You could then also remove the entire RUN gpuci_conda_retry install line from this PR.
  2. Whoops. For this comment, I was referring to this statement in the PR body: So when we launch the docker image with JupyterHub we can easily update the examples at runtime. I was thinking about the following for both nightly and stable images:
  • Nightlies - nightlies are rebuilt every night, so therefore the notebooks are usually up to date when you pull the latest nightly
  • Stable images - whenever we publish a stable release with new images, the active development branch changes. So if BUILD_BRANCH is equal to branch-0.19 for instance, then after the 0.19 release, all new development will occur on the branch-0.20 branch. I'm not sure how/if nbgitpuller will handle this branch change. I assume (though I didn't confirm this in any way) that nbgitpuller will only update the notebooks according to the branch in which it was originally invoked with (i.e. branch-0.19 in the previous example). If this is the case, then I think that nbgitpuller would only be useful in the instance where users download an early nightly container and want to update their notebooks to the latest notebooks for the RAPIDS version of their current container (and not necessarily the latest overall notebooks, which could potentially be on a new branch). Hopefully that makes sense. nbgitpuller is small enough that it's probably fine to include in our images regardless, but all of this is moot until they get that submodules PR merged 🙂 .

@ajschmidt8 ajschmidt8 changed the base branch from branch-0.18 to branch-0.20 April 22, 2021 21:11
@ajschmidt8
Copy link
Member

@jacobtomlinson, I'm not sure if this PR is still relevant with the recent overhaul of the Docker images.

Can we close this?

If these features will still work with the new images, can you adapt your PR to the changes that were recently merged?

@jacobtomlinson
Copy link
Member Author

I think these changes would still be useful, but the exact use case that was motivating this in 2021 is gone now so I'll probably follow up if/when it comes up again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants