Allow building more complex profile_list templates

[yuvipanda]

The profile_list HTML / CSS was a single string embedded into the python code,
and modifications were allowed as a single traitlet that had to be a fully complete jinja2
template.

This becomes difficult to manage very quickly, and means we lose out on all the compositional
benefits of using jinja2 templates.

This PR will:

• Split the existing HTML / CSS string into a separate HTML & CSS file
• Switch to FilesystemLoader, so we can split out the one big HTML / CSS file into
  multiple smaller files and compose them together
• Add the config additional_profile_form_template_paths to allow admins to set extra paths where templates can be found. This
  allows admins to customize the profile_list form easily without having to shove everything
  into one file. This is stolen from JupyterHub's JupyterHub.template_paths.
• Add a pre-commit hook to autoformat the default templates we ship
• Add some docs on how admins can continue to customize the profile_list template by setting
  the string, but also recommends using the directory based approach for more complex changes.
  This allows the change to be backwards compatible.

[manics]

https://setuptools.pypa.io/en/latest/userguide/datafiles.html#accessing-data-files-at-runtime recommends using importlib.resources instead of relying on direct filesystem access.

[yuvipanda]

@manics it looks like that would only work on python 3.10+ without adding an external dependency. From https://setuptools.pypa.io/en/latest/userguide/datafiles.html#accessing-data-files-at-runtime

importlib.resources was added to Python 3.7. However, the API illustrated in this code (using files()) was added only in Python 3.9, [3] and support for accessing data files via namespace packages was added only in Python 3.10 [4] (the data subdirectory is a namespace package under the root package mypkg). Therefore, you may find this code to work only in Python 3.10 (and above). For other versions of Python, you are recommended to use the importlib-resources backport which provides the latest version of this library.

I'd say it's ok to let this be, maybe add a note to remove it once we are python 3.10+ only?

[yuvipanda]

@manics the alternative is to add the additional dependency, which we currently don't have. I just checked to see if any jupyterhub or kubespawner dependency transitively includes that - turns out the answer is no. If you strongly prefer, I will add that dependency?

[yuvipanda]

(am working through the pre-commit situation now)

[manics]

If there's no easy fix then leave it as it is unless anyone else has thoughts

[yuvipanda]

hmm, not sure if the test failures are related.

[yuvipanda]

Per @consideRatio the test failures are unrelated #721 (comment)****

[consideRatio]

Per @consideRatio the test failures are unrelated #721 (comment)****

@yuvipanda the failures I meant would have shown up during the action-k3s-helm step, not in pytest. It seems that the failure seen here is recurring for the py37 env with oldest dependencies.

But, they seem to be indepent of this PR as when I triggered the test workflow in the main branch, I saw the failures as well: https://github.com/jupyterhub/kubespawner/actions/runs/4824271639

[yuvipanda]

@consideRatio makes sense. Let's investigate that separately from this PR?

[consideRatio]

@yuvipanda yes I agree, separate investigation.

I'm considering an conditional dependency for importlib_resources. I've seen Min do similar things in several places.

Python version constrained requirement, that is conditionally imported.

Reading this not confident on the implications or meaning, I would feel at ease knowing we do that.

It is strongly recommended that, if you are using data files, you should use importlib.resources to access them.

[yuvipanda]

@consideRatio ok, I've expanded the scope of this PR a little bit, and in the process got rid of the manual template loading :)

[yuvipanda]

Alright, I've completely removed the usage of __file__ from here!

[consideRatio]

I love seeing these as standalone files, and I'm happy to see a jinja autoformatter we can use with pre-commit!

I think the additional_profile_form_template_paths sounds reasonable, thinking about the option of simply having profile_form_template_paths and letting its @default(...) function evaluate something. Maybe not though, then its far easier to override it when you want to append to it when configuring this via YAML based config rather than Python based config. So, maybe this is the way to go as it is.

Are the .html / .css files solely extracted and autoformatted - but not changed?

[yuvipanda]

@consideRatio I have not changed the contents of the templates, so there should be no visual differences. Just extracted and autoformatted.

[consideRatio]

I think this LGTM. @manics what do you think?

---

@yuvipanda can you update the PR description to mention that you add the new config option, and maybe consider if the title can highlight what is new for the jupyterhub admin reading this changelog? It sounds like a bugfix/maintenance PR in a way as it mentions "robustly".

[yuvipanda]

@consideRatio how about now for the PR title & description?

[manics]

I haven't reviewed the latest code but I'm happy to go with @consideRatio

Are we happy that the wheel package correctly contains these extra files (from python -mbuild .), or can we rely on pyproject.toml/the build system to just "do the right thing"?

[yuvipanda]

@manics hmm, I see it in the sdist but not wheel! Investigating.

[yuvipanda]

@manics turns out hatch doesn't read MANIFEST.in, so I've included it separately in the wheel now. Good catch!

[consideRatio]

Nice work @yuvipanda !!!

[yuvipanda]

THANKS A LOT, @consideRatio!