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

Ensure hub_url is configured before use #1641

Closed berghaus closed 1 year ago

berghaus commented 1 year ago

This PR follows the discussion in discourse post 18215.

Problem: the configuration of BinderHub fails if hub_url is not set.

Solution: set a default value for the hub_url in the helm chart. Check that hub_url is set before use.

I tested the helm chart (as per the documentation), but am unsure how to test the change to binderhub_config.py properly.

welcome[bot] commented 1 year ago

Thanks for submitting your first pull request! You are awesome! :hugs:
If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly. welcome You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! :wave:
Welcome to the Jupyter community! :tada:

manics commented 1 year ago

You can just set the default to "" in https://github.com/jupyterhub/binderhub/blob/6b16f8d3888263be8b3a1ccbc8f63734b55627ec/binderhub/app.py#L512-L513 e.g. https://github.com/jupyterhub/binderhub/blob/6b16f8d3888263be8b3a1ccbc8f63734b55627ec/binderhub/app.py#L404-L417

berghaus commented 1 year ago

You can just set the default to "" in

https://github.com/jupyterhub/binderhub/blob/6b16f8d3888263be8b3a1ccbc8f63734b55627ec/binderhub/app.py#L512-L513

Unfortunately, that won't address the issue. The error is raised because an unset configuration value is retrieved, not the BinderHub parameter that the configuration value is meant to set:

https://github.com/jupyterhub/binderhub/blob/ddba2322d9a3f8d8d1a91e405f4ceec488071740/helm-chart/binderhub/files/binderhub_config.py#L58

I proved this to myself like so:

# Sample configurable:
from traitlets.config.configurable import Configurable
from traitlets.config import get_config
from traitlets import Int, Unicode
from urllib.parse import urlparse

class MyClass(Configurable):
    name = Unicode(config=True)
    number = Int(config=True)

# Create a configuration
c = get_config()
c.MyClass.number = 10
try:
    urlparse(c.MyClass.name)
except AttributeError:
    print("using config item produced error")

# construct class instance:
m = MyClass()
try:
    urlparse(m.name)
except AttributeError:
    print("using instane property produced error")

print("test done")

which produces:

using config item produced error
test done
welcome[bot] commented 1 year ago

Congrats on your first merged pull request in this project! :tada: congrats Thank you for contributing, we are very proud of you! :heart: