Make port checking optional

[ryshoooo]

This PR introduces additional parameter for the server proxy configuration, which specifies whether the port availability checking is necessary to perform or not. This is useful to disable if the application is already running (started by something else) and the user just wants to create a simple proxy for it (not start it through proxy directly).

[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:

[ryanlovett]

Hi @ryshoooo, if/when I need to start the proxied service manually I use the anonymous proxy URL, e.g. {base_url}/proxy/{port}/{proxied_path} rather than {base_url}/{name}/{proxied_path}. Is this method not desired or sufficient? Is this what you mean by, "(not start it through proxy directly)" ?

[ryshoooo]

Hi @ryshoooo, if/when I need to start the proxied service manually I use the anonymous proxy URL, e.g. {base_url}/proxy/{port}/{proxied_path} rather than {base_url}/{name}/{proxied_path}. Is this method not desired or sufficient? Is this what you mean by, "(not start it through proxy directly)" ?

Hi @ryanlovett, yes it is exactly not desired :) To give you a specific example of what we're facing, we're using Z2JH with Kubernetes and we wish to provide code-server for each single-user environment. The old way we used to do this, was that we pre-installed code-server into the single-user base image and then used j-s-p the way it's supposed to be used, i.e. the command starts code-server and we open it up at some random open port. We're moving away from this solution in order to have even more micro-service architecture in place, so now we start the code-server as an additional extra container in the single-user pod during spawning. This approach has quite a few advantages, mainly that now we can do upgrades of code-server without touching the single-user base image. Anyway, the users got very much used to the fact that URL user/{username}/code points to the vscode, so we'd like to preserve that with the new setup :)

[manics]

I think the general idea sounds good. Did you consider any other options, e.g. something like managed_service=True|False to control whether JSP starts the service or not, which is easier to debug (since you're not in an indeterminate state)

[ryshoooo]

I think the general idea sounds good. Did you consider any other options, e.g. something like managed_service=True|False to control whether JSP starts the service or not, which is easier to debug (since you're not in an indeterminate state)

A little bit yes, I just decided not to go this way fully just yet :) I merely just wanted to have something quickly working without too many changes to the codebase. Which this does for me :) But I do like your suggestion and I do think it is the better solution with the managed_service parameter. But then it makes command parameter conditionally optional, which I'm not sure how I feel about it, feels like spaghetti. So that's mainly why I didn't go that way :)

If you think it's the right way, I can try and turn it into managed_service parameter. It'll take a bit of time :)

[manics]

But then it makes command parameter conditionally optional, which I'm not sure how I feel about it, feels like spaghetti.

That's a good point! Maybe we could make the command argument optional instead of adding the new flag. If command is empty just setup the port-fowarding? @ryanlovett what do you think?

[ryanlovett]

@manics That seems reasonable to me.

[ryshoooo]

I've added the changes, removed the check_port parameter, and made the command argument optional. However, the tests seem to be failing due to missing jupyter-notebook binary? Seems a bit unrelated to this PR right?

[ryanlovett]

Thanks @ryshoooo ! Yes, I'm guessing we need to add jupyter-notebook as a new dependency before running tests. (in a separate PR)

[manics]

The tests should be fixed by https://github.com/jupyterhub/jupyter-server-proxy/pull/340

Would you mind adding a test for this new configuration, e.g. based on https://github.com/jupyterhub/jupyter-server-proxy/blob/099fce21feed4c5c762740dea2d34c5ca548c5b3/tests/resources/jupyter_server_config.py#L39-L41 https://github.com/jupyterhub/jupyter-server-proxy/blob/099fce21feed4c5c762740dea2d34c5ca548c5b3/tests/test_proxies.py#L141-L150

For the backend service you can just connect to one of the existing ones, so effectively the new configuration would be an alias.

[ryshoooo]

@manics

Thanks for the guidance :) I've added an additional test for the new functionality.

[ryshoooo]

Looks like the pipeline is failing again, but the logs are a bit vague. Does anybody have any idea what's going on?

[consideRatio]

Looks like the pipeline is failing again, but the logs are a bit vague. Does anybody have any idea what's going on?

Please ignore them. I'm not sure, but its related to jupyter_server 2 being released and that the jupyterlab extension isn't doing well with it.

[ryshoooo]

This LGTM as it is now. I added two commits, one updating docs and another one relocating a return statement to happen sooner rather than later when an unmanaged process is involved.

• The test failures should be unrelated, see #362.

@ryshoooo does this look good to you as it is now?

Hi @consideRatio

Yes this looks totally fine to me, thanks a lot for your help! :)

[welcome[bot]]

Congrats on your first merged pull request in this project! :tada: Thank you for contributing, we are very proud of you! :heart:

[consideRatio]

Thank you @ryshoooo! ❤️ 🎉