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

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

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

zph
Copy link

@zph zph commented Apr 28, 2024

Closes: #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

  • Give the PR a descriptive title.
  • 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.
  • 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 added 2 commits April 28, 2024 15:53
…pec installs

Closes: denoland#20744

Adds ability to specify the kernelspec location for deno kernel with
--directory flag

Continued default behavior installs in user's kernelspec folder via
jupyter shelling out:
`deno jupyter --install`

Advanced installs can specify their own path:
`deno jupyter --install --directory ~/.kernelspec_custom_location/deno`

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.
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

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

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

cli/args/flags.rs Outdated Show resolved Hide resolved
cli/tools/jupyter/install.rs Outdated Show resolved Hide resolved
Comment on lines +68 to +70
if !dir.ends_with("deno") {
dir.push("deno");
}
Copy link
Member

Choose a reason for hiding this comment

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

Good for the first pass 👍 for the future we might want to derive the name from the current version - eg. in Deno v1.43 this would add deno-v1.43 and for Deno v1.44.2 it would add deno-v1.44. That would make it very handy managing multiple Deno installations.

Copy link
Author

Choose a reason for hiding this comment

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

@bartlomieju I can try it out and confirm it works, if we want it later, may as well start with that behavior.

@bartlomieju bartlomieju added this to the 1.44 milestone May 5, 2024
zph and others added 4 commits May 6, 2024 08:16
This reverts commit 5286f0a.
Co-authored-by: Bartek Iwańczuk <biwanczuk@gmail.com>
Signed-off-by: Zander Hill <zander@xargs.io>
@zph
Copy link
Author

zph commented May 6, 2024

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.

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.

Install kernel without requiring users to install jupyter
2 participants