Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review public/private methods and classes #909

Open
manics opened this issue Jun 1, 2023 · 6 comments
Open

Review public/private methods and classes #909

manics opened this issue Jun 1, 2023 · 6 comments
Labels
enhancement New feature or request

Comments

@manics
Copy link
Member

manics commented Jun 1, 2023

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

class CustomSpawner(SystemdSpawner):

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:

  • Rename private classes or methods with _ following common Python practices
  • Document what we consider public
@manics manics added the enhancement New feature or request label Jun 1, 2023
@manics manics mentioned this issue Jun 1, 2023
1 task
@minrk
Copy link
Member

minrk commented Jun 1, 2023

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
Copy link
Member Author

manics commented Jun 1, 2023

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
Copy link
Member

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

class UserCreatingSpawner(SystemdSpawner):
"""
SystemdSpawner with user creation on spawn.

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

@consideRatio
Copy link
Member

@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
Copy link
Member Author

manics commented Jul 5, 2023

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
Copy link
Member

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants