Add custom hub package & config hooks #360
Conversation
yuvipanda
changed the title
yuvipanda
force-pushed
the
repo2dockerspawner
branch
4 times, most recently
from
c80c2e9
to
1f15299
Compare
|
Adding these hooks to help set up https://discourse.jupyter.org/t/would-a-the-littlest-binder-be-useful/1041/28. I've locally hacked this up to work, so am trying to upstream it all. |
There was a problem hiding this comment.
This looks awesome @yuvipanda ❤️ . I left a couple of comments inline, but feel free to ignore them if you think they are too far-fetched (I'm not very familiar with the plugin code and I might not fully understand the whole context).
| @@ -46,3 +53,11 @@ def test_config_hook(): | |||
| data = yaml.load(f) | |||
|
|
|||
| assert data['simplest_plugin']['present'] | |||
|
|
|||
| def test_tmpauthenticator(): | |||
| """ | |||
There was a problem hiding this comment.
I would probably name this test_tmpauthenticator_enabled or test_jupyterhub_config_hook (I'm not very good at naming things though 👀 ) but the name as it is makes me think we're testing the authenticator.
There was a problem hiding this comment.
Good catch! I've renamed it!
| @@ -208,6 +208,7 @@ def ensure_jupyterhub_package(prefix): | |||
| 'jupyterhub-firstuseauthenticator==0.12', | |||
| 'jupyterhub-nativeauthenticator==0.0.4', | |||
| 'jupyterhub-ldapauthenticator==1.2.2', | |||
| 'jupyterhub-tmpauthenticator==0.6', | |||
There was a problem hiding this comment.
From what I understand, when a user wants to custom its juputerhub_config using tljh_custom_jupyterhub_config in order to use something that's not directly installed by TLJH, they could would use tljh_extra_hub_pip_packages to install it first.
If this is the case, I believe tmpauthenticator package should/could come from tljh_extra_hub_pip_packages.
There was a problem hiding this comment.
I've split this part of the PR to #365 separately, and based this PR on that. So we have two PRs:
- Add tmpauthenticator by default to TLJH #365 that adds tmpauthenticator
- This PR, which adds two hooks with tests for both of them. There is a test for
tljh_extra_hub_pip_packagesthat imports the 'there' package, and the test fortljh_custom_jupyterhub_configthat sets tmpauthenticator to be the default authenticator.
How does that sound?
yuvipanda
force-pushed
the
repo2dockerspawner
branch
from
1f15299
to
fc7b6ae
Compare
Is popular enough we should let people use it by default
Required when installing additional authenticators or spawners
This lets extensions directly control how JupyterHub is configured
yuvipanda
force-pushed
the
repo2dockerspawner
branch
from
fc7b6ae
to
b7f72a9
Compare
|
Thanks for the review, @GeorgianaElena! I've updated the PR to reflect your suggestions. Let me know what you think :) #365 should probably be merged before this though - this PR 'depends' on that (it includes some commits from there). So once #365 gets merged, the diff on this PR should stop showing the contents of #365. This might be confusing - let me know if I can help clarify that. |
|
Excellent @yuvipanda 🎊. Once the tests pass I'll try to merge the PRs in the order you suggested :D |
Add hook for installing additional hub packages
Add hook for additional
jupyterhub_config.pyentries
Add / update documentation
Add tests