jupyterhub / kubespawner

Kubernetes spawner for JupyterHub
https://jupyterhub-kubespawner.readthedocs.io
BSD 3-Clause "New" or "Revised" License
546 stars 304 forks source link

Refactor out profile_list related logic from spawner.py #775

Open consideRatio opened 1 year ago

consideRatio commented 1 year ago

@GeorgianaElena wrote in https://github.com/jupyterhub/kubespawner/pull/774#pullrequestreview-1606707512:

[...] 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 commented 1 year ago

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