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.

Sign up for GitHub

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

Jump to bottom

Prevent load of disabled (or disabling) extension modules #1178

Closed
wants to merge 1 commit into from
Closed

Prevent load of disabled (or disabling) extension modules #1178

wants to merge 1 commit into from

Conversation

kevin-bates
Copy link
Member

This pull request addresses the issue raised in #1177 by skipping the load of extension modules that are disabled or are in the process of being disabled via the tooling.

There were some display decisions that I took the liberty to apply, but we can discuss them further if necessary.

  1. the - Writing config: entry has been indented to align with the other entries.
  2. The Package xxx took yyys to import message has also been indented to align with the other entries the tool produces. Since this entry is also displayed at application startup, it appears indented. (I personally find that indentation stands out more in the startup console and would prefer that and the nicer tool alignment than having it left justified.)
  3. I print that validation is NOT happening when the extension is disabled or getting disabled - so that users won't ask why they don't see the validation.

There are two test failures that are the result of the tests not being sensitive to the current state of the module. I will address these but wanted to get a consensus on this approach first.

Also, can a given module contain multiple extensions? It seems like the tooling is only enabling/disabling at the module level, but I may be missing something.

Here are some outputs of the various commands:

List extensions:

$ jupyter server extension list                         
Config dir: /Users/kbates/.jupyter

Config dir: /opt/miniconda3/envs/server-dev/etc/jupyter
    jupyter_server_mathjax enabled
    - Validating jupyter_server_mathjax...
    - Package jupyter_server_mathjax took 0.0010s to import
      jupyter_server_mathjax  OK
    jupyter_server_terminals enabled
    - Validating jupyter_server_terminals...
    - Package jupyter_server_terminals took 0.0075s to import
      jupyter_server_terminals 0.4.3 OK
    nbdime enabled
    - Validating nbdime...
    - Package nbdime took 0.0152s to import
      nbdime 3.1.1 OK

Config dir: /usr/local/etc/jupyter

Disable mathjax:

$ jupyter server extension disable jupyter_server_mathjax
    - Package jupyter_server_mathjax took 0.0012s to import
    - Package jupyter_server_terminals took 0.0081s to import
    - Package nbdime took 0.0150s to import
Disabling: jupyter_server_mathjax
    - Writing config: /opt/miniconda3/envs/server-dev/etc/jupyter
    - Validation of jupyter_server_mathjax skipped due to acton to disable.
    - Extension successfully disabled.

List extensions, post-disable:

$ jupyter server extension list                          
Config dir: /Users/kbates/.jupyter

Config dir: /opt/miniconda3/envs/server-dev/etc/jupyter
    jupyter_server_mathjax disabled
    - Validation of jupyter_server_mathjax skipped.
    jupyter_server_terminals enabled
    - Validating jupyter_server_terminals...
    - Package jupyter_server_terminals took 0.0083s to import
      jupyter_server_terminals 0.4.3 OK
    nbdime enabled
    - Validating nbdime...
    - Package nbdime took 0.0150s to import
      nbdime 3.1.1 OK

Config dir: /usr/local/etc/jupyter

Start server (with disabled):

$ jupyter server               
[I 2023-01-15 08:56:11.331 ServerApp]     - Package jupyter_server_terminals took 0.0074s to import
[I 2023-01-15 08:56:11.347 ServerApp]     - Package nbdime took 0.0150s to import
[I 2023-01-15 08:56:11.350 ServerApp] jupyter_server_terminals | extension was successfully linked.
[I 2023-01-15 08:56:11.350 ServerApp] nbdime | extension was successfully linked.
[I 2023-01-15 08:56:11.680 ServerApp] jupyter_server_terminals | extension was successfully loaded.
[I 2023-01-15 08:56:11.830 ServerApp] nbdime | extension was successfully loaded.
[I 2023-01-15 08:56:11.834 ServerApp] Serving notebooks from local directory: /Users/kbates/repos/oss/jupyter/jupyter_server
[I 2023-01-15 08:56:11.834 ServerApp] Jupyter Server 2.2.0.dev0 is running at:
[I 2023-01-15 08:56:11.834 ServerApp] http://localhost:8888/?token=dffea54d162b7d3704002136bc52219efde8ab72ad7ac850

Enable mathjax:

$ jupyter server extension enable jupyter_server_mathjax
    - Package jupyter_server_terminals took 0.0080s to import
    - Package nbdime took 0.0149s to import
Enabling: jupyter_server_mathjax
    - Writing config: /opt/miniconda3/envs/server-dev/etc/jupyter
    - Validating jupyter_server_mathjax...
    - Package jupyter_server_mathjax took 0.0010s to import
      jupyter_server_mathjax  OK
    - Extension successfully enabled.

Start server (with enabled):

$ jupyter server                                        
[I 2023-01-15 08:57:12.792 ServerApp]     - Package jupyter_server_mathjax took 0.0011s to import
[I 2023-01-15 08:57:12.799 ServerApp]     - Package jupyter_server_terminals took 0.0069s to import
[I 2023-01-15 08:57:12.815 ServerApp]     - Package nbdime took 0.0157s to import
[I 2023-01-15 08:57:12.819 ServerApp] jupyter_server_mathjax | extension was successfully linked.
[I 2023-01-15 08:57:12.822 ServerApp] jupyter_server_terminals | extension was successfully linked.
[I 2023-01-15 08:57:12.822 ServerApp] nbdime | extension was successfully linked.
[I 2023-01-15 08:57:13.145 ServerApp] jupyter_server_mathjax | extension was successfully loaded.
[I 2023-01-15 08:57:13.147 ServerApp] jupyter_server_terminals | extension was successfully loaded.
[I 2023-01-15 08:57:13.274 ServerApp] nbdime | extension was successfully loaded.
[I 2023-01-15 08:57:13.277 ServerApp] Serving notebooks from local directory: /Users/kbates/repos/oss/jupyter/jupyter_server
[I 2023-01-15 08:57:13.277 ServerApp] Jupyter Server 2.2.0.dev0 is running at:
[I 2023-01-15 08:57:13.278 ServerApp] http://localhost:8888/?token=b72021ad41800ad853ec431adeda7bacd1e852281f610078

Resolves: #1177

@codecov
Copy link

codecov bot commented Jan 15, 2023 •
edited
Loading

Codecov Report

Base: 80.10% // Head: 39.30% // Decreases project coverage by -40.80% ⚠️

Coverage data is based on head (e674ba1) compared to base (0edaf7c).
Patch coverage: 20.00% of modified lines in pull request are covered.

Additional details and impacted files 
@@             Coverage Diff             @@
##             main    #1178       +/-   ##
===========================================
- Coverage   80.10%   39.30%   -40.81%     
===========================================
  Files          68       68               
  Lines        8153     8162        +9     
  Branches     1606     1609        +3     
===========================================
- Hits         6531     3208     -3323     
- Misses       1196     4752     +3556     
+ Partials      426      202      -224
Impacted Files Coverage Δ
jupyter_server/extension/serverextension.py 53.28% <0.00%> (-31.85%) ⬇️
jupyter_server/extension/manager.py 68.69% <83.33%> (-23.66%) ⬇️
jupyter_server/extension/utils.py 53.33% <100.00%> (-35.56%) ⬇️
jupyter_server/terminal/__init__.py 0.00% <0.00%> (-100.00%) ⬇️
jupyter_server/terminal/handlers.py 0.00% <0.00%> (-100.00%) ⬇️
jupyter_server/terminal/api_handlers.py 0.00% <0.00%> (-100.00%) ⬇️
jupyter_server/terminal/terminalmanager.py 0.00% <0.00%> (-100.00%) ⬇️
...pyter_server/services/contents/largefilemanager.py 11.11% <0.00%> (-72.73%) ⬇️
jupyter_server/services/contents/fileio.py 21.87% <0.00%> (-68.23%) ⬇️
...upyter_server/services/contents/filecheckpoints.py 22.35% <0.00%> (-67.65%) ⬇️
... and 46 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@blink1073
Copy link
Contributor

Thanks for working on this!

It seems to me like there should be a top level bullet for each package and then align everything about it under that, like this:

$ jupyter server extension list                          
Config dir: /Users/kbates/.jupyter

Config dir: /opt/miniconda3/envs/server-dev/etc/jupyter
   -  jupyter_server_mathjax disabled
      Validation of jupyter_server_mathjax skipped
   - jupyter_server_terminals enabled
      Validating jupyter_server_terminals...
      Package jupyter_server_terminals took 0.0083s to import
      jupyter_server_terminals 0.4.3 OK
   - nbdime enabled
     Validating nbdime...
     Package nbdime took 0.0150s to import
     nbdime 3.1.1 OK

Config dir: /usr/local/etc/jupyter

Also note the removal the period after "skipped", for consistency.

@kevin-bates
Copy link
Member Author

Closing in favor of #1180

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Jupyter Server imports extensions even when they are disabled, slowing down startup
2 participants
@kevin-bates @blink1073