Add configuration option to allow restart on graceful exit

[dpwrussell]

With this change, if a process has previously exited gracefully, then it can be started up again. The setup function in the implementing Python package would look something like this:

def setup_jsptest():
    return {
        'command': ["something", "which", "gracefully", "exits"],
        'environment': {},
        'launcher_entry': {
            'title': 'jsptest',
            'icon_path': os.path.join(os.path.dirname(os.path.abspath(__file__)), 'icons', 'jsptest.svg')
        },
        'allow_restart_on_graceful_exit': True
    }

I made it optional because the trigger for starting up these service is a request to the application server and I can imagine a scenario where a server process has gracefully exited, but browser windows to that location remain open and the user does not wish the server to restart.

If this general approach seems ok, then I could also turn allow_restart_on_graceful_exit to accept a function so that programmatic control over whether the app should restart is possible.

Very happy to have suggestions for the name of the config option name instead of allow_restart_on_graceful_exit.

Resolves #213.

[welcome[bot]]

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. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly. 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:

[steverweber]

Personally i would rather see: auto_restart and ignore the exit code. Because if something is failing we don't want to keep retrying forever and potentiality flooding the other end. Also sometimes retrying can be process intensive.

Never the less; the option to prevent auto restarting is good,. I can always do something like start_server.sh || true for my personal desire.

[steverweber]

confirmed that

        with (await self.state['proc_lock']):
            if 'proc' in self.state:
                if self.state['proc'].returncode == 0: 
                    del self.state['proc']

works.

[dpwrussell]

Personally i would rather see: auto_restart and ignore the exit code. Because if something is failing we don't want to keep retrying forever and potentiality flooding the other end. Also sometimes retrying can be process intensive.

I think we might want to address that issue separately (I agree it's important and should be configurable) because restarting of processes that have not gracefully exited is handled by simpervisor. We probably need @yuvipanda to weigh in on the feasibility of rolling all the restart options into a single parameter/function given that this works in different ways depending on if it's a graceful exit or not.

@yuvipanda It would be great to get your input on this PR in the general case as well! Particularly on if it would be good (as I think it would be) to make it possible for the allow_restart_on_graceful_exit (or whatever it ends up being called) to be a function.

[consideRatio]

@yuvipanda and @manics, if you have capacity to allocate some time to review this PR I think it would be appreciated.

[manics]

I've merged in master, and pushed a test to this branch. I'm undecided on whether to merge now with the parameter name allow_restart_on_graceful_exit, or whether we should deal with the considerations in https://github.com/jupyterhub/jupyter-server-proxy/pull/215#issuecomment-707147807

[manics]

What do you think about changing the property name to something like restart_policy, auto_restart, .... and making it a string? Initially it could be restricted to either None/'', or on-success to keep the existing behaviour in this PR, but in future we could then add other string values- this will be easier than adding mroe boolean properties for each case.

My suggestion of on-success is taken from the systemd Restart= option, we could optionally add some of the other values such as always or on-failure in future: https://www.freedesktop.org/software/systemd/man/systemd.service.html#Restart= @consideRatio @dpwrussell @yuvipanda ?

If we want to go this way I can push direct to this branch, or open a new PR.

[consideRatio]

@sk1p mentioned this PR in gitter:

We are planning on using jupyter-server-proxy on systems with limited resources. Users can already stop the spawned processes from the spawned web GUI, but when trying to access them again afterwards, they are not restarted, as the proxy "thinks" they are still running.

@manics sorry for not following up with you on this, I think I lack a bit too much experience with this repo to make a decision on its API. @yuvipanda, if you find time, perhaps you could provide feedback to @manics about the proposal in https://github.com/jupyterhub/jupyter-server-proxy/pull/215#issuecomment-793608345?

[yuvipanda]

Thanks for the ping again, @consideRatio! I think restart_policy with possible string options is the way to go! It'll also match what k8s, docker, systemd, etc do.

[manics]

When should the process be restarted? Straightaway in simpervisor (always_restart https://github.com/jupyterhub/simpervisor/blob/1b3c3ad861bdfc7c3bd63c3412a3a68fafd5f7b0/simpervisor/process.py#L20)?

Or only on demand?

[sk1p]

Or only on demand?

For our use case, restart on-demand would work best: a user can decide to stop a web service they don't need anymore, and it won't take up any resources, until they explicitly restart it.

[elmedmouad]

Is the automatic restart already working in case a process is missing or got killed? I'm using jupyter-server-proxy to execute code-server but if the process gets killed I keep getting the 500 error message and the code-server process is never restarted... Is there a fix that exists currently so that my server is restarted when the proc isn't running or do I need to wait for this PR to be included ?

[sk1p]

Is the automatic restart already working in case a process is missing or got killed? I'm using jupyter-server-proxy to execute code-server but if the process gets killed I keep getting the 500 error message and the code-server process is never restarted... Is there a fix that exists currently so that my server is restarted when the proc isn't running or do I need to wait for this PR to be included ?

For me, automated restart after killing works even without this patch - have you checked that all processes terminate (if there are more than one), and there isn't a left-over process somehow?