Support Jupyverse as a single-user server #4520
Conversation
|
@minrk How do you feel about this PR? |
There was a problem hiding this comment.
I think it's sensible to support it, but the PR needs to be updated, since it currently changes the default server to jupyverse.
The defaults and priority should be kept as they are, but adding a new value for JUPYTERHUB_SINGLEUSER_APP=jupyverse so it's opt-in seems AOK.
Make sure to add a test matrix entry with this env so it's exercised.
davidbrochart
force-pushed
the
jupyverse
branch
2 times, most recently
from
003e8e5
to
6b3d4a0
Compare
davidbrochart
force-pushed
the
jupyverse
branch
2 times, most recently
from
70ed684
to
649cb9d
Compare
davidbrochart
force-pushed
the
jupyverse
branch
from
649cb9d
to
d1cf683
Compare
|
I skipped some tests that I see as jupyter-server specific. |
jupyterhub/tests/test_singleuser.py
Outdated
| @@ -167,6 +173,10 @@ async def test_disable_user_config(request, app, tmpdir, full_spawn): | |||
| ) | |||
| assert r.status_code == 200 | |||
|
|
|||
| if IS_JUPYVERSE: | |||
| # the remaining is jupyter-server specific | |||
| return | |||
There was a problem hiding this comment.
Rather than returning, this should verify that no jupyverse config files are loaded from $HOME.
There was a problem hiding this comment.
I still think this is jupyter-server specific. Jupyverse uses the Asphalt application framework where configuration files are passed explicitly, so there is no notion of looking up directories for configuration files.
There was a problem hiding this comment.
Ah, then I'd just update the comment to say that jupyverse never loads any configuration from the home directory, and the skip can move to the top of the test, since there's nothing to check for jupyverse.
| @@ -63,6 +65,8 @@ async def test_singleuser_auth( | |||
| await user.spawn(server_name) | |||
| await app.proxy.add_user(user, server_name) | |||
| spawner = user.spawners[server_name] | |||
| if IS_JUPYVERSE: | |||
| spawner.default_url = "/lab" | |||
There was a problem hiding this comment.
What happens if this is missing? Is it because the default url for jupyverse is not /tree?
There was a problem hiding this comment.
Yes, just like JupyterLab has no /tree endpoint.
jupyterhub/tests/test_singleuser.py
Outdated
| @@ -202,6 +212,9 @@ def assert_not_in_home(path, name): | |||
| async def test_notebook_dir( | |||
| request, app, tmpdir, user, full_spawn, extension, notebook_dir | |||
| ): | |||
| if IS_JUPYVERSE: | |||
| pytest.skip("jupyter-server specific test") | |||
There was a problem hiding this comment.
This test is not jupyter-server specific, it's verifying if the root directory config is respected. The mechanism of the test (specifically the use of the test extension utility) may need different handling, but the API calls to the contents API should produce the same results.
There was a problem hiding this comment.
Jupyverse has no root directory config (yet), what do you think about re-enabling this test if/when there is support for it?
There was a problem hiding this comment.
@minrk what do you think about merging this?
.github/workflows/test.yml
Outdated
| @@ -176,6 +182,13 @@ jobs: | |||
| if [ "${{ matrix.jupyter_server }}" != "" ]; then | |||
| pip install "jupyter_server==${{ matrix.jupyter_server }}" | |||
| fi | |||
| if [ "${{ matrix.jupyverse }}" != "" ]; then | |||
| pip install "git+https://github.com/davidbrochart/jupyverse.git@jupyterhub#egg=fps_auth_jupyterhub&subdirectory=plugins/auth_jupyterhub" | |||
There was a problem hiding this comment.
I think this PR's been merged? Can we depend on 0.2.1 now? If so, 👍 to merge with the updated install command.
There was a problem hiding this comment.
I think this PR's been merged?
Ah no, you're right. I'll make a jupyverse release and update the install command.
There was a problem hiding this comment.
@minrk it should be good to go now.
|
Thanks! |
|
Thanks, do you plan to release soon? |
|
Not immediately, we have some things to finish first. Hopefully before too long, though. |
With jupyter-server/jupyverse#335, this will allow to use Jupyverse as a single-user server.