deno icon indicating copy to clipboard operation
deno copied to clipboard

feat(cli/tools/jupyter) Add --directory flag to control where jupyter kernelspec installs

Open zph opened this issue 9 months ago • 4 comments

Closes: https://github.com/denoland/deno/issues/20744

Adds ability to specify the kernelspec location for deno's jupyter kernel during install with --directory flag without the requirement of having Jupyter binary on the PATH.

Advanced installs can specify their own path: deno jupyter --install --dir ~/.kernelspec_custom_location/deno (Edited from directory -> dir)

Standard installs continue with default behavior by installing in user's kernelspec folder via jupyter shelling out: deno jupyter --install

In the advanced case deno builds and installs the files directly rather than relying on calling out to jupyter to determine path.

This is useful in the circumstance where jupyter is not on PATH at time of installing deno jupyter, but it is available and used via a wrapping library.

Action Items

  • [x] Give the PR a descriptive title.
  • [x] Ensure there is a related issue and it is referenced in the PR text.
  • [ ] Ensure there are tests that cover the changes.
  • [ ] Ensure cargo test passes.
  • [ ] Ensure ./tools/format.js passes without changing files.
  • [ ] Ensure ./tools/lint.js passes.
  • [x] Open as a draft PR if your work is still in progress. The CI won't run all steps, but you can add '[ci]' to a commit message to force it to. If you would like to run the benchmarks on the CI, add the 'ci-bench' label.

zph avatar Apr 28 '24 22:04 zph

Looks great! @zph do you need any pointers for adding tests for this functionality?

I'll see what I can do and then get feedback. I'll need to find where we're doing a form of end to end testing. So far I only found the struct building tests for JupyterFlags.

zph avatar May 06 '24 15:05 zph

@rgbkrk I wonder how this PR compares to your approach in https://github.com/denoland/deno/pull/23826. Should we do both of them?

bartlomieju avatar May 16 '24 22:05 bartlomieju

@bartlomieju I expect we can close out this one and rely on https://github.com/denoland/deno/pull/23826 instead 👍 .

zph avatar May 16 '24 22:05 zph

We could always factor in adding the --directory flag. Where would you want to install the kernel otherwise, considering frontends still need to know where to pick it up? There are a few other places, though they come down to system vs. user paths. Those are largely covered in https://github.com/runtimed/runtimed/blob/main/runtimelib/src/jupyter/dirs.rs.

rgbkrk avatar May 16 '24 23:05 rgbkrk

I'm closing in favor of the runtimelib approach 🫡

zph avatar May 19 '24 08:05 zph