Fix get_loader returning None when load_jupyter_server_extension is not found (#1193)Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

[cmd-ntrf]

Description

The function extension.utils.get_loader returns None instead of raising an exception when an extension does not define_load_jupyter_server_extension nor load_jupyter_server_extension.

Reproduce

1. Install an extension that does not define load_jupyter_server_extension nor _load_jupyter_server_extension, for example jupyterlab-nvdashboard
2. Launch jupyter lab
   The following exception appears in the log:

     Traceback (most recent call last):
      File "/opt/jupyterhub/lib64/python3.9/site-packages/jupyter_server/extension/manager.py", line 355, in load_extension
        extension.load_all_points(self.serverapp)
      File "/opt/jupyterhub/lib64/python3.9/site-packages/jupyter_server/extension/manager.py", line 229, in load_all_points
        return [self.load_point(point_name, serverapp) for point_name in self.extension_points]
      File "/opt/jupyterhub/lib64/python3.9/site-packages/jupyter_server/extension/manager.py", line 229, in <listcomp>
        return [self.load_point(point_name, serverapp) for point_name in self.extension_points]
      File "/opt/jupyterhub/lib64/python3.9/site-packages/jupyter_server/extension/manager.py", line 222, in load_point
        return point.load(serverapp)
      File "/opt/jupyterhub/lib64/python3.9/site-packages/jupyter_server/extension/manager.py", line 148, in load
        return loader(serverapp)
    TypeError: 'NoneType' object is not callable

Expected behavior

The user should get an AttributeError exception instead of a TypeError, making it clear that _load_jupyter_server_extension is missing for the extension being loaded.

Context

The problem was introduced by PR #742, when a default value of None was configured if gettattr could not find load_jupyter_server_extension. The change was made to satisfy Flake8.

Fix implemented in this PR

Remove the default value for gettatr in get_loader and set the comment # noqa B009 to keep Flake8 satisfied.

[welcome]

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.

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! 🎉

[codecov]

Codecov Report

Base: 79.14% // Head: 80.41% // Increases project coverage by +1.27% 🎉

Coverage data is based on head (7fa7c57) compared to base (f3fe168).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1193      +/-   ##
==========================================
+ Coverage   79.14%   80.41%   +1.27%     
==========================================
  Files          68       68              
  Lines        8117     8119       +2     
  Branches     1584     1583       -1     
==========================================
+ Hits         6424     6529     +105     
+ Misses       1280     1175     -105     
- Partials      413      415       +2     

Impacted Files	Coverage Δ
jupyter_server/extension/utils.py	97.87% <100.00%> (+8.98%)	⬆️
...ter_server/services/kernels/connection/channels.py	61.06% <0.00%> (-1.11%)	⬇️
jupyter_server/base/handlers.py	78.82% <0.00%> (+0.58%)	⬆️
jupyter_server/utils.py	85.46% <0.00%> (+5.81%)	⬆️
jupyter_server/serverapp.py	79.94% <0.00%> (+7.80%)	⬆️
jupyter_server/services/shutdown.py	100.00% <0.00%> (+23.07%)	⬆️

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]

Thanks @cmd-ntrf! It looks like one test needs to be updated:

=========================== short test summary info ============================
FAILED tests/extension/test_utils.py::test_get_loader - AttributeError: 'object' object has no attribute 'load_jupyter_server_extension'

[cmd-ntrf]

While fixing the tests, I realized get_loader would never raised ExtensionLoadingError, so I splitted the try/except in two. I have also added a test that a deprecated warning was raised, instead of ignoring the warning. Finally, the test for when the module is missing both possible function name now correctly validates that an ExtensionLoadingError is raised.

[blink1073]

The link failure is unrelated but the lint failure is: tests/extension/test_utils.py:2:8: F401 warnings imported but unused.

[blink1073]

Thank you!

[welcome]

Congrats on your first merged pull request in this project! 🎉

Thank you for contributing, we are very proud of you! ❤️

[cmd-ntrf]

I originally stumbled on this issue while using jupyter-server 1.23.5, so this patch should probably be backported to 1.x.

[blink1073]

@meeseeksdev please backport to 1.x

[lumberbot-app]

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

1. Checkout backport branch and update it.

git checkout 1.x
git pull

2. Cherry pick the first parent branch of the this PR on top of the older branch:

git cherry-pick -x -m1 89b35f4640a49b0e8443476c9b09f4096deb42f6

3. You will likely have some merge/cherry-pick conflict here, fix them and commit:

git commit -am 'Backport PR #1193: Fix get_loader returning None when load_jupyter_server_extension is not found (#1193)Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>'

4. Push to a named branch:

git push YOURFORK 1.x:auto-backport-of-pr-1193-on-1.x

5. Create a PR against branch 1.x, I would have named this PR:

"Backport PR #1193 on branch 1.x (Fix get_loader returning None when load_jupyter_server_extension is not found (#1193)Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.