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

Possibility to use pre-running code-server instead of starting a new one #71

Closed
wants to merge 2 commits into from

Conversation

k-popov
Copy link

@k-popov k-popov commented Nov 19, 2024

Sometimes code-server is already running (e.g. in sidecar container with Jupyter singleuser in Kubernetes). For these cases there is no need to start a new code-server process, it's possible to just proxy requests to TCP port or UNIX socket of already running code-server.

@k-popov
Copy link
Author

k-popov commented Nov 20, 2024

I've also added an option to disable launcher entry with env variable. This is useful when using pre-started code-server and for example certain users are not supposed to have editor available. For them code-server may be set to not run but there is no easy way to disable loading of the entire jupyter-vscode-proxy package (e.g. it's built into Docker image). In this case a non-functional launcher may be hidden from JupyterLab interface.

@manics
Copy link

manics commented Nov 20, 2024

jupyter-server-proxy can already connect to running servers, an extension isn't necessary:
https://jupyter-server-proxy.readthedocs.io/en/latest/arbitrary-ports-hosts.html

@k-popov
Copy link
Author

k-popov commented Nov 20, 2024

That's true. But in this case the user has no button in JupyterLab interface to access VSCode. Making users manually type something like /proxy/8000/ to access the editor is not very user-friendly, IMO.

@xhochy
Copy link
Collaborator

xhochy commented Nov 20, 2024

It looks like this is a totally different code path. If you really want this, you could either add it to your Jupyter config directly or maybe make a separate package.

@manics
Copy link

manics commented Nov 20, 2024

If you just want a launcher you could look at
https://jupyter-app-launcher.readthedocs.io/

@k-popov
Copy link
Author

k-popov commented Nov 20, 2024

I see this patch as addition with full backward compatibility support. I believe this is especially useful for JH running in Kubernetes where "one container - one process" approach is preferred. if following this concept, running a separate code-server process in the same pod with jupyter's singleuser process is not good.

The change doesn't seem to be enough for yet another jupyter vscode proxy package. I was thinking that having one well-maintained package is better that having 1000 similar and unsupported packages with minimal differences.

I don't understand why but I could not make jupyter-server-proxy work with traitlets - although the snippet was loaded during startup I didn't have path /vscode/ available (404 response). But that's not to be discussed in scope of this PR.

Summing the above up I believe this patch is a backward-compatible functionality for not-so-rare case of running Jupyterhub in Kubernetes. But of course it's up to maintainers to decide if the patch is worth accepting or not.

@manics
Copy link

manics commented Nov 20, 2024

If the code path in your patch is used is there anything specific to code-server, or is it just providing a nice URL path and launcher?

If there's nothing specific I think that's a good reason to make a new generic extension (or adapt jupyter-app-launcher) with configurable URL paths/launcher icons, otherwise we run into the problem of needing the equivalent of this PR in every other jupyter-server-proxy extension too.

@k-popov
Copy link
Author

k-popov commented Nov 20, 2024

is there anything specific to code-server

Then you're probably right: the patch only allows to run VSCode inside Jupyter in another way. And that's the only thing specific to code-server. Even this single package is able to run two "flavors" of code-server and not is split into two packages each running its own "flavor". The patch is adding one more "flavor" as I see it. I thought that if jupyter-server-proxy supports specifying exact port and empty command, that's the case which this support was designed for.

Actually this entire package behaviour may configured using Jupyter app launcher (which actually seems to be a really useful jupyter add-on, huge thanks for the hint!). So there are already different ways to implement the same - I will definitely not create one more package.

Thanks for review!

@k-popov k-popov closed this Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants