server extensions not loaded when installed to different locations

[jbeezley]

If two extensions are installed, one with flag --user and another with --sys-prefix, then they are both listed by jupyter nbextension list, but only one is actually loaded when the server starts. I suspect the configuration objects are being over-written rather than merged.

See the following issue for more details: https://github.com/OpenGeoscience/geonotebook/issues/70

[takluyver]

Are they both listed as enabled?

[jbeezley]

Yes, for example:

$ jupyter nbextension list
Known nbextensions:
  config dir: /home/jbeezley/.jupyter/nbconfig
    notebook section
      nbextensions_configurator/config_menu/main  enabled 
      - Validating: problems found:
        - require?  X nbextensions_configurator/config_menu/main
    tree section
      nbextensions_configurator/tree_tab/main  enabled 
      - Validating: problems found:
        - require?  X nbextensions_configurator/tree_tab/main
  config dir: /home/jbeezley/nex/testenv/etc/jupyter/nbconfig
    notebook section
      geonotebook/index  enabled 
      - Validating: OK

$ jupyter serverextension list
config dir: /home/jbeezley/.jupyter
    jupyter_nbextensions_configurator  enabled 
    - Validating...
      jupyter_nbextensions_configurator  OK
config dir: /home/jbeezley/nex/testenv/etc/jupyter
    geonotebook  enabled 
    - Validating...
      geonotebook  OK

I get validation errors, but I think those are unrelated. With this setup, geonotebook's server extension is not initialized. Once I delete the user config at ~/.jupiter, it works again.

[kotfic]

This appears to be due to a condition in traitlets Config.merge method. Config keys are only merged (as opposed to clobbered) if comparing two Config objects. In the case of server extension configurations the nbserver_extensions key is not converted to a Config object during Config.__init__(). This means when nbserver_extensions sub-keys are compared recursively from different jupyter_notebook_config.json files (e.g. from separate --sys-prefix and --user installs) they are being compared as dict rather than <class 'traitlets.config.loader.Config'>. Because of this the outer scope (e.g. ~/.jupyter/jupyter_notebook_config.json ) clobbers the system-prefix scope (e.g. /path/to/virtual_environment/etc/jupyter/jupyter_notebook_config.json).

To illustrate:

jupyter_notebook_config.json installed with --sys-prefix

(jupyter_2063) kotfic@minastirith:~/tmp/jupyter_2063 ⇒ \
cat ~/.venvs/jupyter_2063/etc/jupyter/jupyter_notebook_config.json
{
  "NotebookApp": {
    "nbserver_extensions": {
      "geonotebook": true
    }
  }
}

jupyter_notebook_config.json installed with --user

(jupyter_2063) kotfic@minastirith:~/tmp/jupyter_2063 ⇒ \ 
cat ~/.jupyter/jupyter_notebook_config.json 
{
  "NotebookApp": {
    "nbserver_extensions": {
      "jupyter_nbextensions_configurator": true
    }
  }
}

By the time we complete load_config_file nbserver_extensions will only contain { "jupyter_nbextensions_configurator": True }. Because displaying enabled/disabled plugins looks at each path individually (without merging) each plugin appears to be enabled. But when the config is actually built the --sys-prefix installed configs will not be available.

nbserver_extensions is not converted to a Config object by self._ensure_subconfig() because _ensure_subconfig only converts items that pass the predicate function _is_section_key. nbserver_extensions does not pass this predicate because it is not capitalized.

I have verified that the merge of --user and --sys-prefix nbserver_extension elements works as expected if nbserver_extensions is converted to Nbserver_extensions and a patch is applied to change the trait in the notebook code. This is an unsatisfying but low impact fix. Fully fixing the issue would require refactoring traitlet's Config object which would obviously have much wider impact.

I am happy to put together a PR but I need a little guidance on how to proceed. Thanks!

[takluyver]

Wow, thanks for that thorough investigation!

Fixing it by capitalising a name doesn't seem ideal - especially as that will break all of people's existing config set with the lowercase name. Also, all other config options are lowercase. I'll have a go at thinking of a better way. I'd like to hear @minrk's thoughts too.

[minrk]

The JSON config files for traitlets don't support merging items (setting to an empty dict needs to be possible for override in general). The frontend-specific nbconfig setup does, however, which makes more sense for what we are doing with extensions. I think we probably should have used the same extension system for both sides, rather than traitlets for one and a new setup for the other. This is part of what @SylvainCorlay is proposing in https://github.com/jupyter/enhancement-proposals/pull/21.

I'm not sure of a decent way to fix this beyond moving the nbserver_extensions off of traitlets to the nbconfig loader, and I'm not sure we want to change how we do this yet again for 5.0. Conceivably, we could use that loader on the regular config files, but that seems like asking for trouble. I can sketch it out, if we think it's worth investigating, though.

[minrk]

2108 implements the last idea - use ConfigManager to load a merged config for nbserver_extensions, so it behaves like frontend extensions. It's not my favorite thing, but it gets the behavior we want, and might be worth the ickiness.

[blink1073]

Wait, this means that all of the Dict traitlets in the NotebookApp have this clobbering behavior?

[minrk]

Yes, traits in config are singletons, only class config is merged, so if you have:

c.App.trait = {'a': 1}

at one config level, and

c.App.trait = {'b': 2}

at another, the assignment at the later point takes precedent, and the result is the second assignment {'b': 2}, not the merge of the two.

The best way to think about traitlets config loading is a series of consecutive assignments.

If dict traits were always merged, there would be no way to specify that a value in higher-level config should not be there. Extensions work because the values are bools, and having a False value is equivalent to having removed that value. That's not true of dicts in general.

[blink1073]

In that case I don't think we should special-case the server extensions key amongst the others, as it will only worsen the existing confusion.

[minrk]

@blink1073 would you prefer to move serverextensions over to the nbconfig out of traits, then?

[blink1073]

Yes, with a deprecation of the existing trait that still uses the current config data if it exists (removed in 6.0).

[blink1073]

What if instead we made a separate Dict-based Trait that had different semantics for merging?

[blink1073]

Then we could cleanly mark which parts of the config are merge-able.

[minrk]

That would be a great solution. Unfortunately, the config object has finished merging before it knows which traits each value is for.

@SylvainCorlay has plans for a different extension mechanism (based on #1706) for the separated Jupyter Server, and I'm a bit reticent to add another extension mechanism at the last minute for notebook 5.0 that we are going to deprecate immediately after releasing it, and there doesn't seem to be time between now and and 5.0 to fully implement #1706, since there's lots of changes regarding defaults, etc.

Solving just the merge issue should be pretty simple - either by applying it in a band-aid fashion to the existing field (#2108) or moving it to nbconfig.

[blink1073]

I am :-1: to moving server-specific configuration to nbconfig, since eventually server != notebook. It seems like #2108 is the best short term solution until we have the conf.d approach.