jupyterhub / wrapspawner

Mechanism for runtime configuration of spawners for JupyterHub
BSD 3-Clause "New" or "Revised" License
60 stars 57 forks source link

Fix conflicting key with kubespawner #59

Open Ph0tonic opened 6 months ago

Ph0tonic commented 6 months ago

https://github.com/jupyterhub/kubespawner 6.x is conflicting with this project. Version 6 now includes a way of configuring profiles a bit like wrapspawner but limited to kubernetes. When they introduced this feature, they used the same key to get the right profile from the form. When trying to spawn a process with kubespawner from wrapspawner, it crashes with the following error:

Exception in Future <Task finished name='Task-37' coro=<BaseHandler.spawn_single_user.<locals>.finish_user_spawn() done, defined at
    Traceback (most recent call last):
      File "/usr/local/lib/python3.11/site-packages/tornado/gen.py", line 625, in error_callback
        future.result()
      File "/usr/local/lib/python3.11/site-packages/jupyterhub/handlers/base.py", line 988, in finish_user_spawn
        await spawn_future
      File "/usr/local/lib/python3.11/site-packages/jupyterhub/user.py", line 902, in spawn
        raise e
      File "/usr/local/lib/python3.11/site-packages/jupyterhub/user.py", line 798, in spawn
        url = await gen.with_timeout(timedelta(seconds=spawner.start_timeout), f)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/usr/local/lib/python3.11/site-packages/kubespawner/spawner.py", line 2667, in _start
        await self.load_user_options()
      File "/usr/local/lib/python3.11/site-packages/kubespawner/spawner.py", line 3327, in load_user_options
        self._validate_user_options(profile_list
      File "/usr/local/lib/python3.11/site-packages/kubespawner/spawner.py", line 3132, in _validate_user_options
        profile = self._get_profile(profile_slug, profile_list
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/usr/local/lib/python3.11/site-packages/kubespawner/spawner.py", line 3176, in _get_profile
        raise ValueError
    ValueError: No such profile: kubespawner. Options include: 

This PR fixes this conflict by using a different key name, wrap_profile. I am currently using this fix in production and it's now working fine.

\cc @mbmilligan

welcome[bot] commented 6 months ago

Thanks for submitting your first pull request! You are awesome! :hugs:
If you haven't done so already, check out Jupyter's Code of Conduct. welcome You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! :wave:
Welcome to the Jupyter community! :tada:

Ph0tonic commented 5 months ago

Hi, just willing to bump this up @mbmilligan :+1:

Ph0tonic commented 3 months ago

Hello, any feedback on this issue ? This issue is really blocking any use with KubeSpawner. \cc @minrk

minrk commented 3 months ago

Sorry for the slow response. Changing this key is a breaking change, since launch API calls may use this. Perhaps the right fix is to not pass consumed user_options field(s) to the child spawner? Or namespace them upon passing?

Ph0tonic commented 3 months ago

No problem, thank you very much for your feedback. I understand the need to avoid breaking changes but I am wondering whether the cost to avoid this situation is higher thank just releasing a 2.x version. Namespacing and filtering options could add some code not required making it more complicating to maintain. I am also not sure whether not passing consumed fields or namespacing them could also be considered a breaking change in itself as we change the list of arguments provided to spawners ?