Skip to content

Validate extension name before toggling through CLI #1509

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Darshan808
Copy link
Contributor

Fixes #1507

Description

This PR fixes an issue where running

jupyter server extension disable <nonexistent-extension> 

would misleadingly add the non-existent or misspelled extension to extension list and mark it as disabled, even though it was never installed.

Fix

A validation step is added to check if the extension name is valid and currently installed. If the extension is not found, an error is logged and it is not added to the extension list.

Changes

  • Before
$ jupyter server extension disable jupyter-ai   # should be jupyter_ai

Disabling: jupyter-ai
- Writing config: /<path_to>/etc/jupyter
    - Validating jupyter-ai...
      jupyter-ai  OK
    - Extension successfully disabled.
  • After
$ jupyter server extension disable jupyter-ai

Disabling: jupyter-ai
- Writing config: /<path_to>/etc/jupyter
    - Validating jupyter-ai...
      X Validation failed: The module 'jupyter-ai' could not be found. Are you sure the extension is installed?

This issue was bugging me for a while, and I believe this change enhances the CLI user experience.

@Darshan808
Copy link
Contributor Author

@Zsailer Could I get a review on this ? Also I think I should add a test for this.

@krassowski krassowski added the bug label Apr 7, 2025
@Zsailer Zsailer self-requested a review April 10, 2025 15:27
@krassowski
Copy link
Collaborator

It looks like the downstream test failures are unrelated and will pass once #1510 is merged.

@Zsailer
Copy link
Member

Zsailer commented Apr 14, 2025

Thanks, @Darshan808. This looks good. I went ahead and added a unit test for this as well.

@Zsailer Zsailer force-pushed the add-extension-verification branch from d660015 to aa796ab Compare April 14, 2025 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bad UX when disabling a server extension with a misspelled or non-existent name
3 participants