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

Prevent failed entrypoints from spoiling the launch #443

Merged
merged 2 commits into from
Jun 12, 2024
Merged

Prevent failed entrypoints from spoiling the launch #443

merged 2 commits into from
Jun 12, 2024

Conversation

banesullivan-kobold
Copy link
Contributor

@banesullivan-kobold banesullivan-kobold commented Jan 30, 2024

I am in a JupyterLab environment where many servers are registered under the jupyter-server-proxy entrypoints and occassionaly one of them might fail. Trouble is that if one fails, they all fail and no servers are able to be registered and launched (or maybe just subsequent entrypoints fail to register?).

I'd like for jupyter-server-proxy to gracefully handle failures when loading entrypoints so as to not let one failed entrypoint prevent others from loading.

edit: See demonstration of issue in later comment

Looking forward to any feedback on these changes and if you all think this would do it

Copy link

welcome bot commented Jan 30, 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.
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! 🎉

@yuvipanda
Copy link
Contributor

Hi @banesullivan-kobold!

Am curious - do you have a stacktrace that can help me better understand what kinda failures happen here?

@banesullivan-kobold
Copy link
Contributor Author

Here is an example.

Take the following app.py module that registers two

def setup_app():
   return {
        # Run a simple file server
        "command": ["python", "-m", "http.server", "{port}"],
        "port": 5555,
        "launcher_entry": {
            "enabled": True,
            "title": "My App",
        },
   }


def setup_broken():
   raise NotImplementedError

Then the following setup.py to install and register those servers with jupyter-server-proxy:

import setuptools

setuptools.setup(
    name="app",
    py_modules=["app"],
    entry_points={
        "jupyter_serverproxy_servers": [
            "myapp = app:setup_app",
            "broken = app:setup_broken",
        ]
    },
    install_requires=["jupyter-server-proxy"],
)

To reproduce the issue:

  1. pip install -e . this app.py with its setup.py
  2. Launch jupyterlab: jupyter lab
  3. Notice the following error in the logs
  4. Notice that on the launcher page in JupyterLab, we are missing the entry to the working app.

When launching jupyter lab via jupyter lab on the command line, we see the following error in the logs:

[W 2024-06-11 21:58:08.028 ServerApp] jupyter_server_proxy | extension failed loading with message: NotImplementedError()
    Traceback (most recent call last):
      File "/opt/conda/lib/python3.11/site-packages/jupyter_server/extension/manager.py", line 356, in load_extension
        extension.load_all_points(self.serverapp)
      File "/opt/conda/lib/python3.11/site-packages/jupyter_server/extension/manager.py", line 228, in load_all_points
        return [self.load_point(point_name, serverapp) for point_name in self.extension_points]
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/opt/conda/lib/python3.11/site-packages/jupyter_server/extension/manager.py", line 228, in <listcomp>
        return [self.load_point(point_name, serverapp) for point_name in self.extension_points]
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/opt/conda/lib/python3.11/site-packages/jupyter_server/extension/manager.py", line 219, in load_point
        return point.load(serverapp)
               ^^^^^^^^^^^^^^^^^^^^^
      File "/opt/conda/lib/python3.11/site-packages/jupyter_server/extension/manager.py", line 147, in load
        return loader(serverapp)
               ^^^^^^^^^^^^^^^^^
      File "/opt/conda/lib/python3.11/site-packages/jupyter_server_proxy/__init__.py", line 48, in _load_jupyter_server_extension
        server_processes += get_entrypoint_server_processes(serverproxy_config)
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/opt/conda/lib/python3.11/site-packages/jupyter_server_proxy/config.py", line 100, in get_entrypoint_server_processes
        server_process_config = entry_point.load()()
                                ^^^^^^^^^^^^^^^^^^^^
      File "/app/app.py", line 14, in setup_broken
        raise NotImplementedError
    NotImplementedError
Result Expected
Screenshot 2024-06-11 at 3 02 06 PM Screenshot 2024-06-11 at 3 02 32 PM
Jupyter-server-proxy entirely failed to launch and is not useable I expect jupyter-server-proxy to continue proxying ports and I expect named servers with working configurations to continue to be registered

It is clear that having one single launch config break or be incorrectly specified will entirely break all of jupyter-server-proxy

@banesullivan-kobold
Copy link
Contributor Author

@yuvipanda, I'm curious what I can do to help get this over the finish line? It feels like a pretty critical bug that having any package in the environment incorrectly configuring a named server would prevent all of jupyter-server-proxy from working.

Also, there is a CI failure here now that seems unrelated?

@yuvipanda yuvipanda merged commit 23dde35 into jupyterhub:main Jun 12, 2024
14 checks passed
@yuvipanda
Copy link
Contributor

Sorry for the delayed response, @banesullivan-kobold. I agree this is an important fix to get out. We've fallen behind on review a little, and I'm trying to get back on track.

Thank you for your contribution, and I hope future contributions will get reviewed faster!

the CI failure was unrelated, I restarted CI and it went away

@banesullivan-kobold
Copy link
Contributor Author

Fantastic, thank you for your time reviewing and merging this!!

@yuvipanda yuvipanda added the bug label Jun 13, 2024
@consideRatio consideRatio changed the title Prevent failed entrypoints from spoiling the bunch Prevent failed entrypoints from spoiling the launch Jul 1, 2024
@consideRatio
Copy link
Member

Updated title to reflect suggestion from Simon reviewing the changelog in #488

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.

3 participants