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

Add hub.loadRoles configuration #2405

Merged
merged 3 commits into from
Oct 2, 2021

Conversation

consideRatio
Copy link
Member

@consideRatio consideRatio commented Sep 25, 2021

The motivation for adding a dedicated chart configuration is that
hub.config.JupyterHub.load_roles would end up being overridden easily if
configured from multiple config files. So, having a dictionary
configuration for the same thing can help.

An example for when this would be relevant is the BinderHub helm chart. It defines a hub.service and will want to define a hub.loadRole as well. But then what happens if a user wants to configure the BinderHub Helm chart? The user will end up having overridden BinderHub's config of the JupyterHub Helm chart.

I've deliberated a while on this, and some comments are made in #2386 about it.

The motivation for adding a dedicated chart configuration is that
hub.config.JupyterHub.load_roles would end up being overridden easily if
configured from multiple config files. So, having a dictionary
configuration for the same thing can help.
@consideRatio consideRatio force-pushed the pr/add-hub.loadRoles branch 4 times, most recently from 571ec8d to 3ff3f7a Compare September 30, 2021 11:56
Copy link
Member

@manics manics left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, one typo then this is good to merge!

jupyterhub/schema.yaml Outdated Show resolved Hide resolved
Co-authored-by: Simon Li <orpheus+devel@gmail.com>
@consideRatio
Copy link
Member Author

Thank you for your review efforts @manics! ❤️ 🎉

@manics manics merged commit 509fc47 into jupyterhub:main Oct 2, 2021
consideRatio pushed a commit to jupyterhub/helm-chart that referenced this pull request Oct 2, 2021
jupyterhub/zero-to-jupyterhub-k8s#2405 Merge pull request #2405 from consideRatio/pr/add-hub.loadRoles
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.

2 participants