search

jupyterhub / the-littlest-jupyterhub

Simple JupyterHub distribution for 1-100 users on a single server
https://tljh.jupyter.org
BSD 3-Clause "New" or "Revised" License
1.02k stars 338 forks source link

Review public/private methods and classes #909

Open manics opened 1 year ago

manics commented 1 year ago

Proposed change

With 1.0.0 pending I think we should review what we consider public (people should be free to use/extend/subclass) or private (use at your own risk).

For example, I've just noticed the this spawner has the generic name user_creating_spawner.CustomSpawner https://github.com/jupyterhub/the-littlest-jupyterhub/blob/2d645a7d896da96ec1ce87a4450217b4eefc7126/tljh/user_creating_spawner.py#L9 Should we rename it, or make it private?

Alternative options

Do nothing

Who would use this feature?

Developers and administrators extending TLJH by subclassing classes or reusing functions.

(Optional): Suggest a solution

Either:

minrk commented 1 year ago

Is anything a public Python API in tljh? The Spawner class names are necessarily public, since they are exposed in configuration, but I'm not sure anything in the tljh package is considered an API for user consumption, and I don't think private naming conventions are particularly helpful if that's the case, since it would apply to every module and every function. But if I'm wrong about that, we should definitely be explicit!

manics commented 1 year ago

This was prompted by https://discourse.jupyter.org/t/named-server-and-unique-workdirectories/19668 where I thought a spawner derived from TLJH's spawner was used. However after a bit of digging it looks like it's based on SystemdSpawner, and not TLJH's spawner.

consideRatio commented 1 year ago

Following #912, this is now what it looks like.

https://github.com/jupyterhub/the-littlest-jupyterhub/blob/e6fe1f2ddff6945b725f0401b89be70acf95cd4a/tljh/user_creating_spawner.py#L8-L10

With regards to the UserCreatingSpawner which may be better described as UserCreatingSystemdSpawner, I'd be very happy to "mark it private" or similar.

consideRatio commented 1 year ago

@manics are you okay with not getting this done before 1.0.0, or working this ~today or similar to unblock a 1.0.0 release?

manics commented 1 year ago

I don't think it's a blocker, should we close this?

I don't think we need to rush a 1.0.0 out. It's potentially disruptive due to the large jump in JupyterHub version, do we have enough people available to provide user support? Alternatively we could push a pre-release out and advertise it as ready for testing by experienced admins?

consideRatio commented 1 year ago

I don't think it's a blocker, should we close this?

I figure its a good idea to do this, but I'm out of time and energy to drive it. I'm fine with leaving it open or closing, but don't think we should hold back a release if we don't work it.

Alternatively we could push a pre-release out and advertise it as ready for testing by experienced admins?

Ah, I thought we had to make 1.0.0 directly because of limitations in bootstrap script, but I realize we can do a 1.0.0b1 after testing how it resolves versions - wieeeee thank you @manics for doing that so thoroughly!!!

Okay, let's go for 1.0.0b1!? I really want to get something out though!