Closed cmd-ntrf closed 6 years ago
Thanks very much!
I agree that using rserver makes the setup much easier. I used rsession because rserver wasn't strictly necessary and because rsession comes with both RStudio Desktop and Server. This means that if your notebook is running remotely on a workstation with the desktop version installed, it would be easier to launch this extension.
However, the primary use case for this extension (at least for me) is on JupyterHub where the environment is prepared by an admin. In this case, installing rstudio-server and using rserver is probably easier than using rsession and twiddling the config.
Let me do some testing and I'll get back to this shortly.
The extension now auto-detects the R environment in the same way rserver does. I've removed the PATH and LD_LIBRARY_PATH env as you suggested since I agree that in most cases the extension doesn't need to do that itself.
I'm still looking into rserver.
First, I want to thank you, this extension and nbproxyserver open a whole new world of possible for usage of Jupyter in Advanced Research Computing. I have deployed it at our site and it is already used.
Now to the essential, this is a pretty aggressive patch, so feel free to ask about the pros and cons. It removes any definition of environment variables. I think those should and can be handled before launching Jupyter. At our site, we managed the environment variables through the Environment Modules Lmod, for which I have built a Jupyter extension that works well with this patch.
The patch also replaces rsession by rserver. This avoids the necessity to define any variable related to RStudio or R in the RSessionContext. Also, by using rserver, a user can now restart R through the Session menu. When using rsession, this option of the menu stop rsession and the user has to go back to Jupyter -> New -> RStudio Session again.
That said, there might be something I am missing from the bigger picture that requires to use rsession instead. At first, I thought it would mean that a user could launch multiple RStudio sessions, but it appears that only RStudio Pro allows that.
Finally, I suppose that rserver will be findable through PATH. If it was not the case, one could simply change the cmd value to specify an absolute path. I think we cover more cases this way than by specifying an absolute path that will only be valid on a subset of systems. I have seen the same logic applied in for example the BatchSpawner extension.
Again, thanks for this extension. You can count on me to contribute in a near future.