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

CLI tool for managing JupyterLab extensions #1

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

Conversation

cronan03
Copy link

@cronan03 cronan03 commented Mar 6, 2024

This pull request introduces a CLI (Command Line Interface) tool for managing JupyterLab extensions, designed to enhance developer workflow. The CLI tool, named jupyterlab-builder, provides commands for developing, building, and watching JupyterLab extensions. It leverages the traitlets library for configuration and flexibility.

Key Features:

Develop Command: Allows developers to set up a development environment for lab extensions, facilitating iterative development and testing.
Build Command: Provides a command for building lab extensions, enabling packaging and distribution.
Watch Command: Enables developers to watch lab extensions for changes and automatically rebuild them, improving productivity during development.
BaseExtensionApp: Implements the core logic for parsing commands and delegating them to appropriate subcommands (DevelopLabExtensionApp, BuildLabExtensionApp, WatchLabExtensionApp).
Usage of Traitlets: Utilizes the traitlets library for defining configuration options and managing application state, ensuring flexibility and ease of customization.

Copy link

welcome bot commented Mar 6, 2024

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

This is going in the right direction @cronan03

I added some inline comments. And could you rebase on the main branch as I setup a skeleton structure for the package in #2?

Comment on lines 3 to 4
from jupyterlab.labapp import LabApp
from jupyterlab.commands import build
Copy link
Member

Choose a reason for hiding this comment

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

Those imports should not be needed - it requires to bring them within this package.

For LabApp, it is gonna be tricky. You will need to figure out exactly what part of that class is used to extract it here.

Copy link
Author

@cronan03 cronan03 Mar 23, 2024

Choose a reason for hiding this comment

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

So as I understand there should only be an init.py file along with a separate file for each functionality extracted like build.py, watch.py etc. All other extracted logic , like contents of these above mentioned imports should be a part of the init.py file. Am I correct? Or can I keep the base_extension_app.py and federated_extensions.py file?

Copy link
Member

Choose a reason for hiding this comment

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

You can keep the base_extension_app.py and federated_extensions.py files. You can also create (or not) a dedicated file to bring the code from those imports:

from jupyterlab.labapp import LabApp
from jupyterlab.commands import build

For the LabApp instead of copying all the code (that should be mostly useless), it will be nice to start from an empty class and run the extracted code to see what is actually needed.

My guess is LabApp inherits from LabServerApp that itself inherits of LabConfig. And only attributes of that LabConfig class should be needed here.

from jupyter_core.paths import ENV_JUPYTER_PATH, SYSTEM_JUPYTER_PATH, jupyter_data_dir
from jupyter_core.utils import ensure_dir_exists
from jupyter_server.extension.serverextension import ArgumentConflict
from jupyterlab_server.config import get_federated_extensions
Copy link
Member

Choose a reason for hiding this comment

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

That import should not be needed --> meaning that code should be including within this package

@cronan03
Copy link
Author

cronan03 commented Apr 9, 2024

Hi @fcollonval I have rebased my code on your #2 commit. I have also extracted the methods and classes being used in the federated_extensions.py file. ie
from jupyter_core.paths import ENV_JUPYTER_PATH, SYSTEM_JUPYTER_PATH, jupyter_data_dir
from jupyter_core.utils import ensure_dir_exists
from jupyter_server.extension.serverextension import ArgumentConflict
from jupyterlab_server.config import get_federated_extensions

I will now move on to extracting code used in these imports
from jupyterlab.labapp import LabApp
from jupyterlab.commands import build as per your comments above.

Please let me know if I did this correctly.

@fcollonval
Copy link
Member

fcollonval commented Apr 9, 2024

Thanks a lot @cronan03

I realize my review comment was not specific enough. It is ok to depend on jupyter_core and jupyter_server. The only packages we should not depend on are jupyterlab and jupyterlab_server. Sorry about that as it made you copy to much code in this PR.

@fcollonval
Copy link
Member

If you want to check the imported code, you can also import here the tests for that part of the code. They are located in the file: https://github.com/jupyterlab/jupyterlab/blob/6152e3a5f94d728ccd2727d6d8413f12ddafb669/jupyterlab/tests/test_jupyterlab.py#L156

@cronan03
Copy link
Author

Thanks a lot @cronan03

I realize my review comment was not specific enough. It is ok to depend on jupyter_core and jupyter_server. The only packages we should not depend on are jupyterlab and jupyterlab_server. Sorry about that as it made you copy to much code in this PR.

Thank you for clarifying @fcollonval . I will keep this in mind moving forward. And no worries, I am happy to learn.

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.

None yet

2 participants