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 config to /hub/info endpoint #4442

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Conversation

minrk
Copy link
Member

@minrk minrk commented May 5, 2023

adds JupyterHub.api_include_config list for selecting which config should be included, which includes nothing by default.

instead of adding a new /api/config endpoint, I elected to add a field to the existing /api/info endpoint, which already tells things about the Hub, and config seems like it fits with what's already there.

Key issues:

  • unrecognized classes (e.g. kubespawner Reflectors) will get values from config, not instances, so no default values will be reported, since JupyterHub doesn't know how to find the instances. A discoverability system for configured instances could help here.
  • instantiating Spawners with a mock user may be fiddly, depending on assumptions in the Spawner. This should usually be okay, though, and handled semi-gracefully where failure can occur.

closes #4406

WDYT @yuvipanda?

adds JupyterHub.api_include_config list for selecting which config should be included
@minrk minrk requested a review from yuvipanda May 5, 2023 11:33
Copy link
Contributor

@sgaist sgaist left a comment

Choose a reason for hiding this comment

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

Only minor things, otherwise, it looks good.

Comment on lines +70 to +77
for f in trait_filters:
if "." not in f:
full_class_filters.add(f)
else:
cls_name, _, trait_name = f.partition(".")
if cls_name not in full_class_filters:
cls_filters = class_trait_filters.setdefault(cls_name, set())
cls_filters.add(trait_name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: single letter variable. I would just use filter

all_classes = full_class_filters | set(class_trait_filters.keys())

# track any sections we didn't look at via instances
unused_sections = set() | all_classes
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't all_classes guaranteed to be a set ?
full_class_filters is one even if empty and if so, all_classes is a set of class_trait_filters.keys.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide an API endpoint to get current effective traitlets configuration
2 participants