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

Confusion between extension_name and module name when launching via entrypoints or launch_instance #222

Closed
echarles opened this issue May 6, 2020 · 11 comments

Comments

@echarles
Copy link
Member

echarles commented May 6, 2020

The extension_name is used as jpserver_extensions value when launching via entrypoint or launch_instance

https://github.com/jupyter/jupyter_server/blob/6d909ca928f165fa4f2772b692f08c0a9de4812f/jupyter_server/extension/application.py#L311-L317

This gives issue if the extension_name is different from the module name where the extension resides. To replicate the issue, just change in the example extension_name = "simple_ext1" to extension_name = "new_simple_ext1", launch via python -m simple_ext1 or jupyter simple-ext1 and it will fail.

Of course, if you launch via jupyter server --ServerApp.jpserver_extensions="{'simple_ext1': True}" this will succeed as the jpserver_extensions is overriden via CLI.

Could _jupyter_server_extension_paths be used to use the correct name which would be the module value? In that case, should we add a extension_name to the _jupyter_server_extension_paths dictionaries?

@Zsailer

@Zsailer
Copy link
Member

Zsailer commented May 6, 2020

Thanks for bringing this up, @echarles.

I went back and forth on this issue. I had chosen to set a hard constraint that an extension's name == extension module name. I thought that would reduce some complexity in extensions.

Seeing Lab's usecase, I agree we should relax this constraint.

Here's the current complexity:

  • The extension's package name (currently, assumed to == extension_name) must be listed jpserver_extensions... this is how the server knows where to import the _jupyter_server_extension_paths.
  • The _jupyter_server_extension_paths then encodes:
    • module: the path to the _load_jupyter_server_extension.
    • app: the ExtensionApp class
  • The extension_name in ExtensionApp is used to define the extension's:
    • jupyter_{extension_name}_config files
    • url namespace for static content, i.e. /static/{extension_name}

At the time, it seemed simpler to make the package_name and extension_name match to avoid multiple names for the same extension.

To get around this, we'll need to:

  • add a new property to ExtensionApp, package_name.
  • add "name" field to the _jupyter_server_extension_paths metadata. This should equal extension_name.
  • Switch the jpserver_extensions field in the snippet above (@echarles's comment) to use the new package_name attribute in ExtensionApp.

@echarles
Copy link
Member Author

echarles commented May 6, 2020

add a new property to ExtensionApp, package_name.
add "name" field to the _jupyter_server_extension_paths metadata. This should equal extension_name.
Switch the jpserver_extensions field in the snippet above (@echarles's comment) to use the new package_name attribute in ExtensionApp.

So we would need to have user declarations for package (which is for me synonym to module) and extension name in both class that extends ExtensionApp and in the _jupyter_server_extension_paths? If this is the case, why do we need to duplicate those?

PS: Maybe you can write sample snippets to be sure I understand it correctly.

@Zsailer
Copy link
Member

Zsailer commented May 6, 2020

Technically, "module" and "package_name" are slightly different. "module" is supposed to be the full path to the _load_jupyter_server_extension function—though ExtensionApp's can be lazy with this field because the App is loaded using the "app" field.

So, technically, you should have "module" == "jupyterlab.labapp" and "package_name" == "jupyterlab"

"package_name" is used to find the _jupyter_server_extension_paths function, and "module" is used to find all _load_jupyter_server_extension functions in that package defined by _jupyter_server_extension_paths (remember, a package can have multiple server extensions).

@echarles
Copy link
Member Author

echarles commented May 6, 2020

Brilliant! Thx for clarifying the nomenclature 🥇

I still feel a bit of duplication between all those declarations, but the snippets I would refer to may help to remove that feeling :)

@Zsailer
Copy link
Member

Zsailer commented May 6, 2020

Some snippets to help explain...

ExtensionApp becomes:

class ExtensionApp:

    extension_name = "" # "lab"
    package_name = "" # "jupyterlab"
    ...

    def initialize_server(self):
        ...
        config = Config({
            "ServerApp": {
                # NOTE THE CHANGE TO PACKAGE_NAME HERE
                "jpserver_extensions": {cls.package_name: True}, 
                "open_browser": True,
                "default_url": cls.extension_url
            }
        })

An example of a _jupyter_server_extension_paths:

def _jupyter_server_extension_paths():
    return [
        {
            "module": "path/to/_load_jupyter_server_extension",
            "name": "lab",
            "app": LabApp
        }
    ]

The "name" key is used by serverapp to store loaded extensions.

The config file for enabling the extension becomes:

{
    "ServerApp": {
        "jpserver_extension": {
            "jupyterlab": true
        }
    }
}

@echarles
Copy link
Member Author

echarles commented May 6, 2020

So in ServerApp.jpserver_extension you have to say which package_name you want to enable.

In _jupyter_server_extension_paths you have to say the name and the app... why not only the app and the server would deduce the name the extension object when it would be instantiated? ... but I am sure i miss something...

@Zsailer
Copy link
Member

Zsailer commented May 6, 2020

why not only the app

Many server extensions are not apps. They are just _load_jupyter_server_extension functions.

@blink1073
Copy link
Contributor

Why the change from name -> extension_name for the config file path? Was this to avoid the legacy path resolution?

@Zsailer
Copy link
Member

Zsailer commented May 7, 2020

@blink1073 honestly, I don't have a good answer for that 😆. It was a change I made early on in the development of ExtensionApp's to avoid legacy path resolution, but (I think) I've since patched the places where that would be an issue.

It probably makes sense to switch back... I'll bring it up in the Jupyter Server meeting.

@blink1073
Copy link
Contributor

Unfortunately I had a conflict. I think we should sync up on the jupyter paths logic.

@echarles
Copy link
Member Author

echarles commented Oct 5, 2020

Closing at this confusion has been managed in PRs prior to 1.0.0 release.

@echarles echarles closed this as completed Oct 5, 2020
Zsailer added a commit to Zsailer/jupyter_server that referenced this issue Nov 18, 2022
* add a error modal in the UI when a kernel action is blocked

* make the console log less aggressive

* add initial test for kernelblock plugin

* test ui element for exact match
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

No branches or pull requests

3 participants