Skip to content
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

allow override of CHP defaultTarget, errorTarget #2079

Merged
merged 2 commits into from
Mar 5, 2021

Conversation

minrk
Copy link
Member

@minrk minrk commented Mar 4, 2021

mybinder.org-deploy used to set the whole CHP command, but this is no longer allowed (not quite sure how I feel about that - pros and cons), making it impossible to override these values.

  • defaultTarget ~never makes sense, but it felt weird to leave it out.
    It may make sense in the future if JupyterHub develops an option
    to register itself only as a handler for /hub.
    This hasn't come up yet, as that behavior only makes sense when
    another service is providing the default target
    because requests to /user/foo will not behave sensibly without something handling the default route.
  • errorTarget can be useful in API-only deployments like BinderHub to reduce load on the Hub
    and/or show more friendly messages to users who shouldn't need to know they are using jupyterhub

@consideRatio
Copy link
Member

There is a missing values.yaml entry, and proxy.chp.abcd is used in schema yaml (good IMO) but the deployment reference proxy.abcd (not good IMO).

@minrk
Copy link
Member Author

minrk commented Mar 4, 2021

xref: jupyterhub/jupyterhub#3373 adding the feature making defaultTarget override make sense

- defaultTarget ~never makes sense, but it felt weird to leave it out
- errorTarget can be useful in API-only deployments like BinderHub to reduce load
  and/or show more friendly messages to users who shouldn't need to know they are using jupyterhub
@consideRatio consideRatio merged commit 632f386 into jupyterhub:master Mar 5, 2021
consideRatio pushed a commit to jupyterhub/helm-chart that referenced this pull request Mar 5, 2021
@minrk minrk deleted the custom-error-target branch March 8, 2021 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants