Add idle culler #366
Add idle culler #366
Conversation
There was a problem hiding this comment.
w00t, thanks for working on this important piece!
Copying the file around is not great, I agree. After this PR is done, maybe you can work on extracting it into its own package that can be then used here and in z2jh? I don't think that needs to block this though.
Either way, this looks great. Needs documentation, and I've a few comments on code style. Otherwise, <3.
(Apologies for the delay in reviewing, have been traveling)
GeorgianaElena
force-pushed
the
addIdleCuller
branch
from
f2a2357
to
20374db
Compare
There was a problem hiding this comment.
This is great work! Thanks for adding the tests and the docs :)
I've left a bunch of comments about the docs. Let me know what you think :) I'm also happy to provide more suggested edits there if that'll make things easier for you.
Excited to get this going and get closer to a 1.0 release!
|
Congratulations on your second big PR being merged into TLJH, @GeorgianaElena! <3 \o/ |
|
Look great ! |
|
Yay <3 thanks a lot @yuvipanda |
@yuvipanda, I decided to give this a try :D
However, I have some questions and doubts.
Is there a better way to access the cull_idle_servers.py, without actually copying it to TLJ?
I'm not sure about the test either... Is it ok for the it to be part of test_hub.py since the idle culler is managed by JupyterHub? Also, I'm not sure I'm testing correctly that the user's server was culled.