Closed sdmichelini closed 5 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.
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:
Thanks for the contribution @sdmichelini ! This seems like it would be useful if you want the culler to ignore a proxied app's activity, and I think your change will have the effect you intend.
Could you also please add an entry in doc/source/server-process.md
?
Lastly, it'd also be great if you could add a test case under tests/
.
@ryanlovett Thanks for the prompt review! I've added documentation for the new parameter in the suggested location.
As for the tests is there a config-test pattern to follow that you would recommend? I did a check over the current tests and don't see ones for config where I could plug in other than confirm basic launching of the HTTP server.
@sdmichelini I think you'd need to create a server to proxy that generates "spurious" activity, such as the kind that you want to mask via this PR. resources/httpinfo.py
is a simple server, but for this test implementation you'd need to write one that is more active. That might be the most difficult part.
Then you'd need to add an entry to jupyter_server_config.py
that sets the new configuration option and proxies the busy test server, and write a test function in test_proxies.py
that queries the proxy for activity. It would need to confirm that there is no activity even when you might expect there to be some.
I believe the hub uses /api/status to check for activity. I would imagine last_activity
is not updated frequently, or at all, when you are not proxying your real world busy app, but is updated when that app is running. If so then your test would confirm that, with your new configuration option set, last_activity
is not updated even when the busy test server is running.
This is all easier for me to say than do, so let me know if you have any questions.
Hmm ok, looks more convoluted than I was intending for as I'm not all to familiar with the codebase - what I did to test this was to manually check the hubs activity time when I had a browser session open and my pod was culled.
No worries. Are you able to share info about what app you were proxying? (in the event that we can simulate its behavior)
Yes - https://github.com/coder/code-server - is the app I am proxying.
@ryanlovett any objections to adding this? would be useful to have this feature.
Looking forward for this feature as well.
@sdmichelini No objections. Let me add another reviewer...
@yuvipanda Thank you for taking a look! I have updated the pull-request based on your feedback!
Thanks a lot, @sdmichelini! Looking forward to more PRs from you :)
Allows proxy to be configured if it reports back to JupyterHub. In some cases the proxy will always detect background activity which means that the pod will never get culled. In this case you must disable recording and use an alternative source of activity(outside of this MR).
Closes #471