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

Raise errors on individual problematic extensions when listing extension #1139

Merged
merged 3 commits into from Dec 19, 2022

Conversation

Zsailer
Copy link
Member

@Zsailer Zsailer commented Dec 19, 2022

Fixes #1134. We can backport this to 1.x once it's merged.

This no longer uses an ExtensionManager to load all the extensions as a bunch. Instead, it loads each extension separately, adds a try/catch clause for each individual extension validation, logs the error (if found) for each extension, and doesn't block the validation for other extensions.

Here is an example of the new output:

Config dir: /Users/zsailer/.jupyter

Config dir: /Users/env/etc/jupyter
    jupyter_server_terminals enabled
    - Validating jupyter_server_terminals...
       X Jupyter Server Terminals requires Jupyter Server 2.0+
    Traceback (most recent call last):
      File "/Users/zsailer/Documents/Work/oss/internal/jupyter_server/jupyter_server/extension/serverextension.py", line 336, in list_server_extensions
        extension = ExtensionPackage(name=name, enabled=enabled)
      File "/Users/zsailer/Documents/Work/oss/internal/jupyter_server/jupyter_server/extension/manager.py", line 166, in __init__
        super().__init__(*args, **kwargs)
      File "/Users/env/lib/python3.8/site-packages/traitlets/traitlets.py", line 1238, in __init__
        super_kwargs[key] = value
      File "/Users/env/lib/python3.8/contextlib.py", line 120, in __exit__
        next(self.gen)
      File "/Users/env/lib/python3.8/site-packages/traitlets/traitlets.py", line 1348, in hold_trait_notifications
        value = trait._cross_validate(self, getattr(self, name))
      File "/Users/env/lib/python3.8/site-packages/traitlets/traitlets.py", line 729, in _cross_validate
        value = obj._trait_validators[self.name](obj, proposal)
      File "/Users/env/lib/python3.8/site-packages/traitlets/traitlets.py", line 1132, in __call__
        return self.func(*args, **kwargs)
      File "/Users/zsailer/Documents/Work/oss/internal/jupyter_server/jupyter_server/extension/manager.py", line 175, in _validate_name
        self._module, self._metadata = get_metadata(name)
      File "/Users/zsailer/Documents/Work/oss/internal/jupyter_server/jupyter_server/extension/utils.py", line 55, in get_metadata
        module = importlib.import_module(package_name)
      File "/Users/env/lib/python3.8/importlib/__init__.py", line 127, in import_module
        return _bootstrap._gcd_import(name[level:], package, level)
      File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
      File "<frozen importlib._bootstrap>", line 991, in _find_and_load
      File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
      File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
      File "<frozen importlib._bootstrap_external>", line 843, in exec_module
      File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
      File "/Users/env/lib/python3.8/site-packages/jupyter_server_terminals/__init__.py", line 9, in <module>
        raise RuntimeError("Jupyter Server Terminals requires Jupyter Server 2.0+")
    RuntimeError: Jupyter Server Terminals requires Jupyter Server 2.0+
    jupyterlab enabled
    - Validating jupyterlab...
      jupyterlab 3.5.0 OK
    nbclassic disabled
    - Validating nbclassic...
      nbclassic  OK

@Zsailer Zsailer added the bug label Dec 19, 2022
@codecov
Copy link

codecov bot commented Dec 19, 2022

Codecov Report

Base: 79.87% // Head: 79.93% // Increases project coverage by +0.05% 🎉

Coverage data is based on head (b3d9e36) compared to base (2dd116f).
Patch coverage: 63.63% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1139      +/-   ##
==========================================
+ Coverage   79.87%   79.93%   +0.05%     
==========================================
  Files          68       68              
  Lines        8060     8067       +7     
  Branches     1598     1599       +1     
==========================================
+ Hits         6438     6448      +10     
+ Misses       1198     1197       -1     
+ Partials      424      422       -2     
Impacted Files Coverage Δ
jupyter_server/extension/serverextension.py 85.71% <63.63%> (-1.43%) ⬇️
jupyter_server/serverapp.py 79.05% <0.00%> (+0.08%) ⬆️
...ter_server/services/kernels/connection/channels.py 59.60% <0.00%> (+1.10%) ⬆️

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
Collaborator

I'd say we put the stack trace behind a log.debug, we shouldn't show stack traces to the user by default.

@Zsailer
Copy link
Member Author

Zsailer commented Dec 19, 2022

Done 👍

Copy link
Collaborator

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

Thanks!

@blink1073 blink1073 merged commit 7ff6e52 into jupyter-server:main Dec 19, 2022
@blink1073
Copy link
Collaborator

@meeseeksdev please backport to 1.x

meeseeksmachine pushed a commit to meeseeksmachine/jupyter_server that referenced this pull request Dec 19, 2022
blink1073 pushed a commit that referenced this pull request Dec 20, 2022
…ic extensions when listing extension) (#1141)

Co-authored-by: Zachary Sailer <zsailer@apple.com>
@Zsailer Zsailer deleted the list-ext-err branch January 16, 2024 21:48
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.

serverapp not defined in extension manager
2 participants