Support use with jupyter_server by decoupling notebook dependencies

[GeorgianaElena]

Fixes #177, and (edit by Erik) fixes #176, and fixes 2i2c-org/upstream#28

• I followed the jupyter_server docs on registering an extension to register nbgitpuller as a jupyter_server extension alongside the jupyter notebook extension.
• Switch to using webpack to remove notebook pkg dependency
• Update CI
• Update documentation

Not sure how to test this though 👀 (Situation fixed, thanks to Yuvi :D)

[yuvipanda]

You can just start jupyter server and try a nbgitpuller URL there - that should use jupyter server rather than notebookserver

[GeorgianaElena]

Thanks @yuvipanda! I tried it and observed the following behavior:

• A 404 for static/components/xterm.js-css/index.css and static/terminal/css/override.css
  Related issue: Static files in classic notebook not searched for in site-packages/notebook but instead site-packages/jupyter_server jupyter-server/jupyter_server#451
• same situation explained by @consideRatio here: require not defined - related to use of JupyterLab 3 / jupyter_server? #176

From my understanding, all these files that are reported as "missing" when using jupyter_server, exist in the classic Notebook Server package. Which makes sense, since they are "fronted-related".
However, I don't know how to fix this 😕 Should we change nbgitpuller implementation to not use these notebookserver specific files?

[manics]

Could it be that you need to change some imports from notebook to jupyter_server, and maybe require jupyter_server in setup.py?

[yuvipanda]

Should we change nbgitpuller implementation to not use these notebookserver specific files?

Yep, that's the way forward. Re-using xterm.js from where we do is definitely a notebook-specific implementation detail - see 3714d00 and #4 for other times it needed updating.

Unfortunately, we are also using a template from notebook server to render our page, so this might not be the simplest operation.

[yuvipanda]

The way to do this is to not inherit from page.html. https://github.com/jupyter/notebook/blob/master/notebook/templates/page.html is the source, so we can copy that in and remove our dependency on the notebook package.

In addition, we'll need to make our JS work without the dependencies we inherit from the page.html in the notebook package. I think xterm and jquery are the only ones we need. We could either just copy in the .js files now, although I think this is a great time to introduce webpack here! We couldn't use webpack before because the JS from the notebook page use requirejs...

[GeorgianaElena]

Thanks for the pointers @yuvipanda. I wasn't sure where to start. I'll update this PR once I have something/questions.

[GeorgianaElena]

I've realized that I pushed some commits without saying anything, sorry about that :(

I managed to get this working locally, yay 🎉 I still need to update the CI to actually use this version and then update the docs/README. But I would love some feedback about the implementation.

[yuvipanda]

Some suggestion about removing extra js stuff in the template, but otherwise great! I haven't tested it at all though.

How do you think we can test this against jupyter-server? We need to make sure this still works on both jupyter-server and classic notebook...

[yuvipanda]

Let's remove the ones we don't need? I think we can get rid of a lot of these!

[GeorgianaElena]

I cheated and copied the page.html file from jupyter_server which should have only the things we need. I'm not sure if I'm right though 😬

[GeorgianaElena]

How do you think we can test this against jupyter-server? We need to make sure this still works on both jupyter-server and classic notebook...

@yuvipanda, I parametrized the existing tests in test_api.py to run both with jupyter-notebook and jupyter-server. What do you think?

I have to note one thing though, because I had some troubles making the tests pass on the CI and locally there were no issues.

Without installing a front-end jupyter_server extension (nbclassic or jupyterlab) the GET request to retrieve the repo (GET /git-pull/api?token=secret&repo=https%3A//github.com/binder-examples/jupyter-extension&branch=master) returned a 302 and then a 404 (locally). I'm not very sure of the nbgitpuller internals, but I guess this is expected since jupyter_server has only the backend part, right?

[nibheis]

Hi @GeorgianaElena and @yuvipanda,

We migrated our school infra to jupyterlab 3.1.4 during the summer and now we just realized that nbgitpuller no longer works (arrg, how could we miss that!).

Looks like you've done the hard work to get nbgitpuller to work with jupyter_server... how can we help to get this pull request applied and released ?

[yuvipanda]

@nibheis if you can try this out patch in your setup and tell us any problems you encounter, that will be a big help :)

[yuvipanda]

Without installing a front-end jupyter_server extension (nbclassic or jupyterlab) the GET request to retrieve the repo (GET /git-pull/api?token=secret&repo=https%3A//github.com/binder-examples/jupyter-extension&branch=master) returned a 302 and then a 404 (locally). I'm not very sure of the nbgitpuller internals, but I guess this is expected since jupyter_server has only the backend part, right?

I think the 404 makes sense since it's done after the pulling is successful.

[nibheis]

@nibheis if you can try this out patch in your setup and tell us any problems you encounter, that will be a big help :)

Hi @GeorgianaElena and @yuvipanda,

So, I:

• cloned @GeorgianaElena 's repo (branch "extension" where the commits have been made)
• "pip -e ." installed it
• activated the extension...

and it worked as expected. I could not see any warning in the jupyterlab logs. All looks good to me ; I hope it helps.

I will deploy this branch on all our servers until a release is made.

Many thanks!

[consideRatio]

WIEEEEEEEEEEEE!!! Thank you @GeorgianaElena!!! 🎉 ❤️ 🌻!

Thank you @nibheis, @yuvipanda, and @manics as well!

[yuvipanda]

I've deployed it and it mostly works. Terminal resizing seems awkward tho, and there is this error in the js console:

[manics]

@yuvipanda Could be related to the xtermjs 4 upgrade(?), since the way addons worked was changed. See for example https://github.com/cs01/pyxtermjs/pull/18/files

[nibheis]

I've deployed it and it mostly works. Terminal resizing seems awkward tho, and there is this error in the js console:

Upgraded to jupyterlab 3.1.7, still using the patched nbgitpuller: I don't see any terminal related issues here.

However, I can report a small bug regarding the final redirection (once nbgitpuller has done its job) : if the saved workspace from the previous session has open tabs (i.e. is non-empty), then the final redirection will only happen in the case of a clone operation.

That is: if the repository had previously been cloned AND the saved workspace contains some opened notebooks (for the repo or other), then the final redirection is skipped (the URL visible in the browser changes).

NOTE: if don't think it's linked to nbgitpuller, as I had noticed a similar behavior while adding "next=" parameters.
It looks like a race between the requested redirection (by nbgitpuller - using the "next=..." mechanism) and the workspace state restoration mechanism. Which ever is last will sort of win the final redirection.