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

Refactor out profile_list related logic from spawner.py #775

Open
consideRatio opened this issue Sep 1, 2023 · 1 comment
Open

Refactor out profile_list related logic from spawner.py #775

consideRatio opened this issue Sep 1, 2023 · 1 comment

Comments

@consideRatio
Copy link
Member

consideRatio commented Sep 1, 2023

@GeorgianaElena wrote in #774 (review):

[...] while discussing about the implementation of the unlisted_choice logic with @yuvipanda, there was a general conclusion that would be beneficial to have all the logic related to profiles into its own module, as the spawner.py file has grown a lot and it's became harder to follow.

I agree on the conclusion and opened this issue to reflect this.

Overview of profile_list related code

profile_list is a KubeSpawner specific feature not all Spawners have, but its implemented using Spawner base class provided functionality.

Spawner base class functionality

  1. A user interface
    Spawner.options_form enables KubeSpawner to declare a HTML form to be presented when a server is to be started.
  2. Backend parsing of user interface provided form data
    Spawner.options_from_form enables KubeSpawner to parse form data into a dictionary for later use, stored by JupyterHub into the non-configurable Spawner.user_options the spawner can adjust based on the provided data. If a REST API request is made to jupyterhub to spawn a server, then the parsing is bypassed.

KubeSpawner logic

  1. A user interface
    • KubeSpawner provides a default jinja2 template that renders with profile_list being passed to it, allowing the result to become an Spawner.options_form jinja2 template for JupyterHub to render with for_user, user, etc. being passed to it.
    • KubeSpawner can instead of rendering the default template render another template based on configuring either profile_form_template or additional_profile_form_template_paths.
  2. Backend parsing of user interface provided form data
    • KubeSpawner provides a way to parse the default UI submitted form data via Spawner.options_from_form
  3. Loading selected profiles
    When the UI has been used to pass form data, that has been parsed into the user_options, or when an JupyterHub API request to spawn a server directly passes user_options, then this data is supposed to be accounted for. This is done via an explicit call to KubeSpawner.load_user_options from KubeSpawner._start, which in turn calls _get_profile, _load_profile, and _apply_overrides.

What code to extract?

I think all of the above functions are worth considering extracting from spawner.py and being directly implemented in the KubeSpawner class, but the details to how isn't obvious.

Considerations of a ProfileListSpawnerMixin class

I'd like to consider if we could make a ProfileListSpawnerMixin class and have that solve quite a few things.

  1. We could (?) relocate the profile_list config definition itself to the mixin, but still configure it via c.KubeSpawner.profile_list I think.
  2. We could (?) avoid needing to pass state around by instead referencing self to access the spawner state
  3. We could (?) maybe extract the profile_list functionality from KubeSpawner to other Spawners long term with this strategy

I don't know for that this is viable because I'm unsure about traitlets and Python class inheritance, but I'd love to see an approach like this be considered.

@yuvipanda
Copy link
Collaborator

I started working on this and then realized I had no time so I stopped :(

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

No branches or pull requests

2 participants