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

Add configurable session timeouts for supervisor #5693

Merged
merged 3 commits into from
Dec 10, 2024

Conversation

jmcphers
Copy link
Collaborator

Addresses #5620 by adding an option to specify the number of seconds to wait for the kernel to connect before giving up. This change is mostly on the supervisor side; the client changes here just add the new option and pass it along to the supervisor.

image

The default was 20 (hardcoded) and is now 30 (configurable).

While I was updating the API, I also added some methods/types that will later support #5226 (but aren't currently implemented on the client or server side).

QA Notes

A really easy way to test this is to set the timeout at 1 second, since most kernels do not start up that quickly.

@jmcphers jmcphers requested a review from sharon-wang December 10, 2024 20:01
Copy link
Member

@sharon-wang sharon-wang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

I searched for positronKernelSupervisor.connectionTimeout in Settings and set the timeout to 1 second and the kernel startup timed out.

image

Will we document this on our website and/or the Workbench User Guide docs?

@jmcphers
Copy link
Collaborator Author

Yeah, we'll eventually want to put this in the workbench docs so admins can bump the timeout (or instruct users to do so if they're hitting a wall).

@jmcphers jmcphers merged commit e567e80 into main Dec 10, 2024
5 checks passed
@jmcphers jmcphers deleted the feature/supervisor-connection-timeout branch December 10, 2024 21:53
@github-actions github-actions bot locked and limited conversation to collaborators Dec 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants