-
Notifications
You must be signed in to change notification settings - Fork 0
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
Use opensafely launch rstudio to provide rstudio #92
base: main
Are you sure you want to change the base?
Conversation
2409476
to
211073b
Compare
opensafely launch rstudio:v2 --port 8787 --background --no-browser | ||
elif [[ "$images" =~ "r:v1" ]]; then | ||
opensafely launch rstudio:v1 --port 8787 --background --no-browser | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need a default case? By my reading of this, if someone starts up a new codespace/dev container with a new project that doesn't yet have an R action defined in the project.yaml then they won't get an RStudio session started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, for example if they are just using stata or python, there's no point running rstudio?
Re-attaching to (or rebuilding) the codespace after adding an R action to project.yaml will kick off rstudio, AIUI. That does leave an awkward gap though.
Happy to leave it running regardless, I guess that's the minimal change? But I would suggest we default to v2 if no R actions are currently defined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have an idea for improving the UX of the general "this codespace/.devcontainer is out of date" problem, with a way to "just update everything".
Which will possibly help with resolving this in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not 100% sure on what's the best way. I think the minimal change is best if you want to keep this moving otherwise we (Team REX) can discuss this beginning of next week.
ISTR that when we were doing the first codespaces initiative we decided that non-R users paying the penalty of all the things needed to start up RStudio was worth it for the convenience of R users as the latter is so massively the majority of our users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have updated to run v1 by default, unless v2 is in the project.yaml
This includes detecting which version of the r image a project is using, and launching the correct version. The R autocompletion in vscode editor is still based on the dev container version of R, which is the v1. This frees us from needing to use Rocker as a base image to get rstudio, but changing that is a later task. Fixes #89
60e38ff
to
25c7cce
Compare
This includes detecting which version of the r image a project is using, and launching the correct version.
The R autocompletion in vscode editor is still based on the dev container version of R, which is the v1.
This free's us from needing to use Rocker as a base image to get rstudio, but changing that is a later task.
Fixes #89