-
Notifications
You must be signed in to change notification settings - Fork 15
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
Adopt binderhub
#83
Adopt binderhub
#83
Conversation
dab4e88
to
ec22d1f
Compare
331bc3b
to
4e02472
Compare
Nice, thanks @trungleduc for working on this! Also cc @yuvipanda if you would like to help review and / or provide feedback. This should help with 2i2c-org/binderhub-service#78. |
|
||
On GitHub and GitLab, a user might have to first create an access token with `read` access to use as the password: | ||
|
||
![image](https://user-images.githubusercontent.com/591645/107350843-39c3bf80-6aca-11eb-8b82-6fa95ba4c7e4.png) | ||
|
||
> [!NOTE] | ||
> The `binderhub` build backend does not support configuring private repositories credentials from the interface. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we open a new issue (or comment in #72) to keep track of the current issues with BinderHub and the default git provider?
This is super exciting to me. How can I help? Would it be via code review? Or something else? |
Hi @yuvipanda, a quick look at the implementation would be super helpful for us. |
c.JupyterHub.hub_ip = public_ips()[0] | ||
c.JupyterHub.cleanup_servers = False | ||
c.JupyterHub.spawner_class = Repo2DockerSpawner | ||
if hookimpl: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should document why we have to conditionally define the TLJH hooks?
Probably to allow for the package to be used with the BinderHub chart on Kubernetes? If so, maybe the tljh_repo2docker
should continue to focus on the TLJH integration (be a TLJH plugin) as before, and have a separate package for use with BinderHub?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could also likely be done separately, as the diff for this PR is already quite large.
Also the UI for building environments could eventually be its own jupyterhub-environment-builder
package, and have tljh_repo2docker
depend on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to #84
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @trungleduc, this is great!
I left a small comment to discuss how we would like to architecture the different packages in the long run, if in case this needs a bit more thinking.
Also it looks like it will be easier to test this new feature if the PR is merged and available in a release (which could be a pre-release). So the best for now would probably be to move forward with this, make a new release, and iterate on fixes and adjustments if needed in follow-up PRs.
Also @yuvipanda if you would like to start testing this on some deployments and have some feedback or issues, please don't hesitate to reach out! |
User facing changes
This PR allows
tljh-repo2docker
to usebinderhub
service as the image build backend. By default, it still uses the local docker engine to build image viarepo2docker
, but if thebinderhub_url
is specified, it will switch to usingbinderhub
service instead.New configurations
binderhub_url
: The optional URL of thebinderhub
service. If available,tljh-repo2docker
will use this service to build images.db_url
: The database's connection string.tljh-repo2docker
needs a database to store the image metadata. By default, it will create asqlite
database in the starting directory of the service. To use other databases (PostgreSQL
orMySQL
), users need to specify the connection string via this config and install the additional drivers (asyncpg
oraiomysql
).New environment dialog when using
binderhub
serviceNote
The
binderhub
build backend does not support adding custom build arguments and private repository credentials from the interface.Code changes
binderhub
service.binderhub
build backend.