search

jupyterhub / jupyter-server-proxy

Jupyter notebook server extension to proxy web services.
https://jupyter-server-proxy.readthedocs.io
BSD 3-Clause "New" or "Revised" License
354 stars 147 forks source link

Make config.ServerProcess into a Configurable #507

Open manics opened 1 month ago

manics commented 1 month ago

This isn't useful in the jupyter-server-extension, but it should be useful in https://github.com/jupyterhub/jupyter-server-proxy/pull/501#pullrequestreview-2393608506

An added benefit is this makes it easier to add more parameters in future since the declaration of the class and the handling of defaults is now in the same place.

jwindgassen commented 1 month ago

While at it, we should probably also convert LauncherEntry. Feel free to copy it from my implementation

Edit: I think the self.parent might not work for that. But I don't have much experience with traitlets

manics commented 1 month ago

Thanks, I had LauncherEntry in my local branch, but was hoping to minimise changes. As it happens making it Configurable is the easiest way to set the defaults.

Assuming ServerProcess can be configured externally I don't think LauncherEntry needs to be, it can be initialised from a dict as at present.

manics commented 4 weeks ago

A nice benefit of this is we can autogenerate some of the manually written docs: https://github.com/jupyterhub/jupyter-server-proxy/pull/508

jwindgassen commented 1 week ago

I see why you didn't want to use Configurable for the Launcher Entry. But I think you could just switch to HasTraits as a base class, this should still work for default values. Or make it consistent and make both only use HasTraits, as they are both only used in the ServerProxy class. I think I should still be able to use them in a configurable in my code.

Is there anything else still required here? It would be nice to merge this, so I can update my code it in #501.