search

jupyterhub / binderhub

Run your code in the cloud, with technology so advanced, it feels like magic!
https://binderhub.readthedocs.io
BSD 3-Clause "New" or "Revised" License
2.54k stars 388 forks source link

Fix the default `use_registry` value #1597

Closed manics closed 1 year ago

manics commented 1 year ago

use_registry defaults to try in BinderHub https://github.com/jupyterhub/binderhub/blob/03545a1587f2d781ff4934beac8b8b8a19280754/binderhub/app.py#L261-L270

but the Helm chart implicitly defaults to False: https://github.com/jupyterhub/binderhub/blob/03545a1587f2d781ff4934beac8b8b8a19280754/helm-chart/binderhub/templates/deployment.yaml#L68-L76

Which will lead to a broken deployment.

Also includes a commit to fix the CI upgrade/diff check.

consideRatio commented 1 year ago

Ah, so c.BinderHub.use_registry is one of those software configurations that is required to be known also during chart template rendering. In z2jh c.JupyterHub.base_url is a configuration like that as well.

A big plus if we can make that explicit in comments going onwards as that is a very relevant piece of understanding for refactoring work in general.

I see we have a lot of rate limit errors that also shows up in the main branch tests, due to that, I'll go for a merge here and assume its fine. Maybe we need to resolve the rate limiting issue going onwards, I suspect they have reduced the allowed quota.