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

Fix the examples, migrate to jupyterlab_server #5316

Merged
merged 16 commits into from Sep 19, 2018
Merged

Conversation

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Sep 14, 2018

Depends on jupyterlab/jupyterlab_server#58

This greatly simplifies the example app as well to use as much of jupyterlab_server as we can.

@jasongrout jasongrout added this to the 1.0 milestone Sep 14, 2018
@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Sep 14, 2018

Since we apparently just started publishing jupyterlab_server, I went ahead and changed jupyterlab_launcher to jupyterlab_server.

@jasongrout jasongrout changed the title [WIP] Fix the example app Fix the example app Sep 14, 2018
@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Sep 14, 2018

Ready for review now.

@jasongrout jasongrout changed the title Fix the example app Fix the examples Sep 14, 2018
@jasongrout jasongrout changed the title Fix the examples Fix the examples, migrate to jupyterlab_server Sep 14, 2018
@blink1073 blink1073 closed this Sep 14, 2018
@blink1073 blink1073 reopened this Sep 14, 2018
Copy link
Member

@blink1073 blink1073 left a comment

Thanks!

@blink1073
Copy link
Member

@blink1073 blink1073 commented Sep 14, 2018

Launching this app actually launches the core JupyterLab app, because it is already installed as a server extension on /lab.

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Sep 14, 2018

It definitely launches the example app for me (I can see the difference when I remove plugins like the faq plugin, for example). Nevertheless, I'll move the example app to a different URL to show how it can be done.

@blink1073
Copy link
Member

@blink1073 blink1073 commented Sep 14, 2018

What does "Jupyter serverextension list" show for you?

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Sep 14, 2018

I made the example app be served from a different directory, just to make sure there is no overlap (and also as a useful example of how to do it). As I looked into it further, it seemed that there were some weird interactions, perhaps some files being served by the lab extension, some not.

Depends on jupyterlab/jupyterlab_server#59

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Sep 14, 2018

There are other PRs with the same failing test in the docs component, which doesn't appear to be related to this PR.

@blink1073
Copy link
Member

@blink1073 blink1073 commented Sep 15, 2018

This is still hitting /lab/api/build on startup...

@blink1073
Copy link
Member

@blink1073 blink1073 commented Sep 15, 2018

I bumped the server dep.

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Sep 17, 2018

Coming from this: https://github.com/jupyterlab/jupyterlab/blob/master/packages/application-extension/src/index.tsx#L97

Thanks. I think that's the only url endpoint actually provided by jupyterlab, not jupyterlab_server. I think probably the most elegant solution is figuring out a way to put it in its own plugin that can be included in jlab, but not in custom remixes that don't have the build step.

However, there still is the issue of whether the service manager should be the one to provide the capabilities, or not. To address the issue of the service manager, some options are:

A. the service manager is only for server handlers provided by the notebook server or the jupyterlab_server package, and other server endpoints are on their own. In this case, the service manager can expose some lower-level functions to help with url routing or other things needed in communicating with an endpoint. Then a separate jupyterlab extension handles all communication with the build check endpoint.
B. the service manager provides a registration system for extending itself, and takes care of some basic url routing infrastructure like prepending the lab namespace, etc. The jupyterlab package registers an object to handle communication with the build check endpoint.
C. the service manager actually includes something for managing the build check endpoint, configured with a new page config setting of build_check_url or something, which defaults to empty (i.e., off) for jupyterlab_server.

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Sep 17, 2018

It looks like C. is already implemented: there is a buildAvailable option in the page config that is (should be) checked before attempting to do anything build related. I'll dive deeper.

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Sep 17, 2018

This is still hitting /lab/api/build on startup...

@blink1073, I'm not seeing this. I did:

conda create -n jlabex notebook
conda activate jlabex
pip install jupyterlab_server
git clone git@github.com:jupyterlab/jupyterlab.git
cd jupyterlab
git checkout -b jasongrout-app master
git pull https://github.com/jasongrout/jupyterlab.git app
cd examples/app
npm install
npm run build
python main.py --debug

I checked both the server log and the browser console Network tab, and didn't see it accessing any /lab urls.

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Sep 17, 2018

Just to be sure, I also did a build with pip install -ve of the jupyterlab source, then doing jlpm install; jlpm run build; jlpm run build:examples, and I still didn't see any hits of any /lab urls.

@jasongrout jasongrout mentioned this pull request Sep 17, 2018
@blink1073
Copy link
Member

@blink1073 blink1073 commented Sep 18, 2018

pip install -ve does not install the server extension. I ran the same as above, but used pip install -v . and I'm getting: [D 02:39:59.253 ExampleApp] 200 GET /lab/api/build?1537256398238 (::1) 1013.00ms

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Sep 18, 2018

Okay, thanks. I confirm that I'm seeing that too. Diving in...

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Sep 18, 2018

I think what is going on is:

A. The new example app inherits from the notebook app
B. The notebook app loads whatever server extensions are enabled
C. The jupyterlab server extension by default says that the build endpoint is available (i.e., adds that information to the webapp settings page_config
D. When the example app writes out the page config, it also writes out this buildAvailable page config variable, which the frontend then interprets.

In the long run, I think the rebuild logic should be a completely separate plugin in jlab, so example apps like this just don't include that logic in the js side, so it doesn't matter if it is in the build config or not. In the short term, I'll explicitly remove the buildAvailable entry from the page config in the example.

jasongrout added 2 commits Sep 18, 2018
If the jupyterlab server extension happens to be installed and enabled, it will automatically add the build check to the page config data, so we override it.
@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Sep 18, 2018

@blink1073, can you try again? My last commit explicitly sets the build availability to false, overriding what the jlab server extension is doing.

This makes sure that, for example, any server extensions are not loaded, including the JupyterLab server extension that might modify the page_config_data.
@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Sep 18, 2018

@blink1073, just pushed a better approach - to disable the Jupyter config system for the example app, so that whatever is installed or configured for the server does not affect the example app.

@blink1073
Copy link
Member

@blink1073 blink1073 commented Sep 19, 2018

Looks great, thanks!

@blink1073 blink1073 merged commit d600547 into jupyterlab:master Sep 19, 2018
1 of 2 checks passed
@blink1073 blink1073 removed this from the 1.0 milestone Sep 28, 2018
@blink1073 blink1073 added this to the 0.35 milestone Sep 28, 2018
@blink1073 blink1073 mentioned this pull request Sep 28, 2018
31 tasks
@lock lock bot locked as resolved and limited conversation to collaborators Aug 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants