add notebook_dir as trait to be deprecated

[Zsailer]

This fixes issue #35.

[SylvainCorlay]

• With this behavior, there may be an issue when setting root_dir, and reading notebook_dir.

• Maybe an observer on root_dir setting the notebook_dir trait through set_trait (bypassing validation) would be better.

• However, we should be careful about incompatible setting of one vs the other?

[Zsailer]

Oops, that should be an observe decorator.

[Zsailer]

Ah, you're right. If root_dir changes, notebook_dir doesn't change.

I think setting notebook_dir in the root_dir observer should fix that. Why bypass validation? They should end up having the same validation logic right?

[Zsailer]

How about adding a

if self.notebook_dir != change['new']:
    self.notebook_dir = change['new']

to the root_dir observer, and

if self.root_dir != change['new']:
    self.root_dir = change['new']

to the notebook_dir observer?

[Zsailer]

This ensures the traits are equal.

[SylvainCorlay]

I need to do some experiments. But it is not trivial.

[Zsailer]

totally agree it's not trivial.

[Zsailer]

I added the check above if you want to test it. Just checks notebook_dir and root_dir are equal. If not, update the other trait.

[Zsailer]

@SylvainCorlay

The pattern used in the classic notebook is to observe the deprecated trait for changes; when it is changed 1) warn the user and 2) allow it to change the new trait. Changing the new trait does not update the deprecated trait.

Two examples in the classic notebook are base_project_url and server_extensions.

Should I simply follow that pattern here?

@observe('notebook_dir')
def _update_notebook_dir(self, change):
    self.log.warning(_('deprecation_warning'))
    self.root_dir = change['new']

[SylvainCorlay]

Should I simply follow that pattern here?

I think we can do that.

[krinsman]

Will deprecating and then eventually removing notebook_dir make this incompatible with JupyterHub, and thus prevent jupyter_server from being used as a backend for jupyterhub-singleuser/SingleUserNotebookApp?

JupyterHub uses it in a pretty essential way, since that's what c.Spawner.notebook_dir hooks into:
https://github.com/jupyterhub/jupyterhub/blob/master/jupyterhub/spawner.py#L467
https://github.com/jupyterhub/jupyterhub/blob/master/jupyterhub/spawner.py#L930

[Zsailer]

Will deprecating and then eventually removing notebook_dir make this incompatible with JupyterHub

@krinsman. We'll have to change this trait in JupyterHub, too.

[Zsailer]

I think we can do that.

👍 @SylvainCorlay merging.