jupyter-rsession-proxy icon indicating copy to clipboard operation
jupyter-rsession-proxy copied to clipboard

Published 20 hours ago • verified publisher icon jupyterhub

Adds support for unix_socket

Open jhgoebbert opened this issue 1 year ago • 3 comments
trafficstars

This PR enables RStudio connections via sockets in combination with this PR for RStudio https://github.com/rstudio/rstudio/pull/14938

jhgoebbert avatar Jul 07 '24 13:07 jhgoebbert

This PR also

  • adds www-thread-pool-size as a possible option which can be set via the environment variable RSERVER_THREAD_POOL_SIZE
  • calls rserver just once to get all supported arguments in _support_args(). This is useful as rserver might be only available through modules which takes some time to load. Therefore, this loading can be reduced.

jhgoebbert avatar Jul 08 '24 06:07 jhgoebbert

yay, thank you for your contribution both here and upstream to rstudio! This is going to unlock a lot of non-containerized use cases - I'm super excited for that to land :)

If you could split the PR into a unix socket one and one with everything else, that would make this easier to review and merge. I would like to hold off on merging the unix socket one until that upstream PR gets merged, but the others should be easier. Would you be open to splitting it up like that?

Thanks!

yuvipanda avatar Jul 12 '24 21:07 yuvipanda

Thanks @yuvipanda . I will split the PR.

The PR for unix-socket support in RStudio is now merged and will be part of the official "Kousa Dogwood" builds expected in 2024.10 : https://github.com/rstudio/rstudio/pull/14938#issuecomment-2243950205

jhgoebbert avatar Jul 23 '24 10:07 jhgoebbert

RStudio v2024.12.0+467 is out: https://github.com/rstudio/rstudio/releases/tag/v2024.12.0%2B467

benz0li avatar Dec 17 '24 09:12 benz0li

I would like to hold off on merging the unix socket one until that upstream PR gets merged, but the others should be easier. Would you be open to splitting it up like that?

@jhgoebbert Could you split the PR into a unix socket one and one with everything else?

Thank you.

benz0li avatar Dec 17 '24 09:12 benz0li

@yuvipanda Or could this be merged as is?

@consideRatio Any objections?

benz0li avatar Dec 19 '24 08:12 benz0li

I tested this PR successfully with RStudio v2024.12.0+467 and

pip install git+https://github.com/jhgoebbert/jupyter-rsession-proxy@feature/sockets

plus setting environment variable RSERVER_USE_SOCKET=1.

benz0li avatar Dec 19 '24 09:12 benz0li

@jhgoebbert @yuvipanda @consideRatio Happy Holidays!

benz0li avatar Dec 21 '24 21:12 benz0li

Happy holidays @benz0li!! ❤️

Sorry for not helping out here now. I'm currently less available for open-source contributions than I've been for many years.

consideRatio avatar Dec 21 '24 22:12 consideRatio

Thanks for testing @benz0li ! I also think it'd be good to split the PR per @yuvipanda's comments. (the unix socket support and the reduction in subprocess invocation)

I agree that it would be great to have this functionality.

ryanlovett avatar Dec 22 '24 05:12 ryanlovett

This PR gets closed as I have splitted the PR like @yuvipanda asked for in https://github.com/jupyterhub/jupyter-rsession-proxy/pull/151#issuecomment-2226401030

  • unix-socket-support: https://github.com/jupyterhub/jupyter-rsession-proxy/pull/159

I will create a PR for the other features as soon as the unix-socket-support is merged.

jhgoebbert avatar Jan 01 '25 20:01 jhgoebbert

The second part of this PR can now be found here

  • minimize overhead: https://github.com/jupyterhub/jupyter-rsession-proxy/pull/160

jhgoebbert avatar Jan 03 '25 13:01 jhgoebbert