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

NotebookApp shim layer #7

Merged
merged 13 commits into from
Mar 16, 2020
Merged

NotebookApp shim layer #7

merged 13 commits into from
Mar 16, 2020

Conversation

Zsailer
Copy link
Member

@Zsailer Zsailer commented Feb 27, 2020

Summary

This PR adds a simple class for shimming traits for apps that are transitioning from NotebookApp (notebook package) to ServerApp (jupyter_server package)

Some traits have moved, so their prefix in jupyter's CLI and config files have changed—e.g. NotebookApp.port has moved to ServerApp.port.

Some traits have duplicated, but no longer inherit their values—e.g. NotebookApp.default_url is different than ServerApp.default_url. Previously, these traits were equal. Now, users must configure their traits separately.

Below, we give a detailed description of what happens when traits are intercepted by this shim layer. The goal is that projects can use this shim layer during an intermediate "transition" period after switching to Jupyter Server. Then, after a short cycle of "deprecation warnings", they can drop this shim layer without any interruption.

Detailed Explanation.

A detailed explanation of how traits are handled:

  1. If the argument is prefixed with ServerApp,
    pass this trait to ServerApp.
  2. If the argument is prefixed with NotebookApp,
    • If the argument is a trait of NotebookApp and ServerApp:
      1. Raise a warning—for the extension developers—that
        there's redundant traits.
      2. Pass trait to NotebookApp.
    • If the argument is a trait of just ServerApp only
      (i.e. the trait moved from NotebookApp to ServerApp):
      1. Raise a "this trait has moved" for the user.
      2. Pass trait to ServerApp.
    • If the argument is a trait of NotebookApp only, pass trait
      to NotebookApp.
    • If the argument is not found in any object, raise a
      "Trait not found." error.
  3. If the argument is prefixed with ExtensionApp:
    • If the argument is a trait of ExtensionApp,
      NotebookApp, and ServerApp,
      1. Raise a warning about redundancy.
      2. Pass to the ExtensionApp
    • If the argument is a trait of ExtensionApp and NotebookApp,
      1. Raise a warning about redundancy.
      2. Pass to ExtensionApp.
    • If the argument is a trait of ExtensionApp and ServerApp,
      1. Raise a warning about redundancy.
      2. Pass to ExtensionApp.
    • If the argument is a trait of ExtensionApp.
      1. Pass to ExtensionApp.
    • If the argument is a trait of NotebookApp but not ExtensionApp,
      1. Raise a warning that trait has likely moved to NotebookApp.
      2. Pass to NotebookApp
    • If the arguent is a trait of ServerApp but not ExtensionApp,
      1. Raise a warning that the trait has likely moved to ServerApp.
      2. Pass to ServerApp.
    • else
      • Raise a TraitError: "trait not found."

@Zsailer
Copy link
Member Author

Zsailer commented Feb 27, 2020

Posting some important documentation here.

The order in which CLI arguments are loaded and passed to their respective classes matters when working with ExtensionApp.

Listing the current steps here.

  1. Args are passed to ExtensionApp launch_instance()
  2. _preparse_for_subcommand() looks for subcommands (i.e. list, stop, etc.)
  3. _preparse_for_stopping_flags looks for flags like --help, etc.
  4. ServerApp instance is created.
  5. ServerApp.initialize() is called.
    1. ServerApp.parse_command_line() is called, parsing args for traits
    2. ServerApp.update_config() is called to update traits based on CLI.
  6. ExtensionApp instance is created.
  7. ExtensionApp.initialize() is called.
    1. ExtensionApp.parse_command_line() is called, parsing args for traits again.
    2. ExtensionApp.update_config() is called again.

there's redundant traits.
2. Pass trait to `NotebookApp`.
- If the argument is a trait of just `ServerApp` only
(i.e. the trait moved from `NotebookApp` to `ServerApp`):
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to clarify, take this construed example:

  • If I start nbclassic, I want the server to serve it from port 8888
  • I want all other jupyter servers to serve from port 9001

How would I configure this?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think you would want to launch two separate servers. Launch nbclassic like so:

jupyter nbclassic --ServerApp.port=8888

then start a second server with

jupyter server --ServerApp.port=9001 --ServerApp.jpserver_extensions=[...]

Aliases can be used in both examples to shorten the CLI. You can also point each server to separate config files with these options set there too.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, so basically the answer is "this is not possible via the config system, please use command arguments". Sure.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, I see. I thought you were asking about running two servers at the same time.

Right—if you have redundant config, the "last write wins". Currently, extensions are sorted alphabetically before they are loaded. So if two extensions configure the server, the last one in the list will override all others.

Copy link
Contributor

@vidartf vidartf Feb 28, 2020

Choose a reason for hiding this comment

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

Oh, I see. I thought you were asking about running two servers at the same time.

I was, but without using command line arguments :)

@Zsailer
Copy link
Member Author

Zsailer commented Feb 28, 2020

I've run into a challenging issue with moving traits from an extension to the ServerApp. There is a circular loading issue that makes shimming traits to ServerApp difficult.

The ServerApp always needs to be initialized and configured before extensions (and their config) is loaded, because many ServerApp traits are passed to the tornado webapp object. If traits change (i.e. an ExtensionApp updates the ServerApp config), the webapp object must be re-instantiate+initialize, losing any previously loaded extensions+config.

@Zsailer
Copy link
Member Author

Zsailer commented Feb 28, 2020

I've run into a challenging issue with moving traits from an extension to the ServerApp.

I've found a way around this, but it will require changes to Jupyter Server. I'll open a PR there soon.

@Zsailer
Copy link
Member Author

Zsailer commented Feb 28, 2020

jupyter-server/jupyter_server#180 enables the ServerApp to discover config in extensions before creating instances of WebApplication and HTTPServer. This enables the shim layer defined here to intercept this config and update traits appropriately.

This shim layer should be working now. Now on to writing tests! 😎

@echarles
Copy link
Member

Working a bit more with this branch, I spot a strange behavior with the jlab examples. The template dir is taken into account but the template is no more found. Maybe something to fix on jlab side, but still this is a notable change in the behavior.

@Zsailer
Copy link
Member Author

Zsailer commented Mar 16, 2020

Merging this, as tests are stable. I'll iterate in future PRs.

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

Successfully merging this pull request may close these issues.

3 participants