Repository navigation
Run rstudioapi::jobRunScript() in a new background console - #17042
Conversation
|
E2E Tests 🚀 Why these tags?
More on automatic tags from changed files. Test count: Linux: 352 tests across 2 shards. |
| { | ||
| "name": "execute_code_in_new_session", | ||
| "summary": "Execute code in a new session", | ||
| "description": "Start a new console session of the caller's runtime in the background, without making it the foreground session, and queue code in it. Returns once the code is queued. The code must be complete; it is run as-is without completeness checks.", |
There was a problem hiding this comment.
Is this best modeled as a separate RPC? There are already cases where the existing execute_code will start a new session, and the existing method offers more control over the session's raise/focus behavior. What do you think about having a special "session id" value for the existing RPC that means "start a new session"?
There was a problem hiding this comment.
This is a very good question! I looked at this in detail one more time, and I do still think that a separate method is a better fit. Just for posterity about why:
- The existing
execute_codehas no session ID parameter. It runs code in the console for the language, starting with the foreground console first, then the last active one. A special value would need a newsession_idparameter that accepts only that one value. - In such a mode, most of the
execute_codecontract would actually work pretty differently. It would return no value, but the job needs the ID of the new session.session_namehas no use for an existing session,allow_incompletedoes not fit (because the job code needs to skip the completeness checks), the language server of the caller cannot reply while R waits for this request, andfocushas no effect. focuscontrols only the input focus. Withfocus: false, the target console still becomes the active console and the foreground session ,but the job type workflow needs a console that does not take the foreground, so a merged method needs a new parameter for this too.- When
execute_codestarts a session, it can join a console that is already starting, and in an untrusted workspace, it can run the code in the console of the caller. The job type workflow needs the opposite, with a new session every time and an error in an untrusted workspace.
If we find we need more control later, we can add optional parameters to the new method (for example, activate), which would not be a breaking change.
| "description": "Start a new console session of the caller's runtime in the background, without making it the foreground session, and queue code in it. Returns once the code is queued. The code must be complete; it is run as-is without completeness checks.", | ||
| "params": [ | ||
| { | ||
| "name": "language_id", |
There was a problem hiding this comment.
If we keep this as-is, do we need to worry about the new session using a different runtime than the initiator, and thus lacking the context expected? (if so we might take a runtime id instead of a language id or have some code to align the initiating session)
There was a problem hiding this comment.
Oh, this is a good catch! If the calling session was not found or was for another language, the code used the preferred runtime and didn't tell anyone about it. I made some changes in f25e476, so now the request fails in these two cases and no session starts. The fallback to the preferred runtime now applies only when no session sent the request.
FWIW the same runtime does not give exactly the same context. The new session does not get the global variables of the caller (i.e. importEnv is not supported yet), the loaded packages, or the options. Also, the new session starts in the workspace folder before the setwd() call. Right now an .Rprofile or renv project in the working directory of the caller has no effect. IIUC this is the same as in RStudio, where a job also starts in a new R process, so not a huge problem for existing users.
|
This works great for me! A couple comments on the RPC design in the UI comm, up to you whether you want to refactor in any way. |
- Addresses posit-dev/positron#6720 - Related to posit-dev/positron/pull/17042 `rstudioapi::jobRunScript()` now works in Positron. ark asks Positron to start a new R console session in the background, and that session runs the script with `source()`. The caller's console stays free while the script runs. For example, `targets::tar_make(as_job = TRUE)` now runs the pipeline in its own console. Before this change, the call failed with "Function runScriptJob not found in RStudio", because ark did not define `.rs.api.runScriptJob()`. ### Changes - Add the `execute_code_in_new_session` frontend method to the UI comm. `crates/amalthea/src/comm/ui_comm.rs` is generated from the Positron contract. - Add `.ps.ui.executeCodeInNewSession(code, sessionName)`. It parses `code` first. If the code does not parse, it stops with an error in the caller's session, and no session starts. It returns the ID of the new session. - Add `.rs.api.runScriptJob()` with the same arguments as in RStudio, so that the `callFun()` of rstudioapi accepts it: - `path`, `name`, `encoding`, and `workingDir` must each be a single string. - The script must exist. If `name` is `NULL` or empty, the session name is the file name. If `workingDir` is `NULL`, the working directory is the folder of the script. The working directory must exist. - ark normalizes both paths in the caller's session, so relative paths use the working directory of the caller. - `importEnv = TRUE` and a non-empty `exportEnv` are not supported. Each one gives a warning, and the job still runs. - The function returns the session ID invisibly. The new session runs this code: ```r { setwd("/path/to/working/dir") source("/path/to/script.R", encoding = "unknown") } ``` ark makes each value with `deparse()`, so quotes and backslashes in paths are safe. The braces make one expression. Positron can split console input into statements and continue after an error. Without the braces, a failed `setwd()` would let `source()` run in the wrong folder. ### Why not `setWorkingDirectory()` I originally wanted to use `ILanguageRuntimeSession.setWorkingDirectory()` from Positron, but it is not a good fit here: - For R, it sends `setwd()` directly to the kernel, outside the console queue. It does not wait for the result, and it continues after an error. The order of `setwd()` and `source()` running would then depend on timing. - It escapes only `\` and `"` while `deparse()` escapes all special characters. - The supervisor rejects it for kernels that do not implement it, such as Python. - It runs after the session starts, so `.Rprofile` and renv still use the workspace folder. A real fix needs a working directory at session start, and Positron does not have this option yet. ### Known limitations - The caller's console is busy until the new session starts and the code is in the queue. This takes some seconds, or even more with a complicated `.Rprofile` or renv. ark needs the reply to return the session ID and to report start errors. - If the user interrupts the caller during this wait, R returns, but the job still starts. - The `.ps.ui.*` functions need an active execute request, so a call from a `later::later()` callback fails. This limit is the same for all `.ps.ui.*` functions currently. - The other rstudioapi job functions (`jobAdd()`, `jobGetState()`, and more) are not part of this change. ### Testing - New integration tests in `crates/ark/tests/integration/rstudioapi.rs` cover the request that ark sends (with a path that contains a space and a quote), `name` and `workingDir`, relative paths, a missing file or folder, bad argument types, the warnings, and code that does not parse. Tests that call `rstudioapi::jobRunScript()` skip when rstudioapi is not installed. Tests that call `.rs.api.runScriptJob()` directly always run. - Run them with `cargo nextest run -p ark --test integration -E 'test(rstudioapi::)'`. They need nextest, because each test needs its own R session. - The expected paths remove the `\\?\` prefix that `canonicalize()` adds on Windows. I did not run the tests on Windows, so so we'll want to especially check the Windows CI result. - To do a manual test in Positron (with the companion PR): 1. Make `foo.R` with `Sys.sleep(10); print(getwd())`. 2. In the R console, run `rstudioapi::jobRunScript("foo.R")`. 3. After some seconds, the call returns. A new console named `foo.R` shows in the session list, and the original console stays selected. You can type in the original console while the script runs, and the new console prints the folder of the script. ### Positron Release Notes #### New Features - `rstudioapi::jobRunScript()` now runs the script in a new R console in the background, so the current console stays free. As a result, `targets::tar_make(as_job = TRUE)` now works (posit-dev/positron#6720). #### Bug Fixes - N/A
rstudioapi::jobRunScript()in a new background console ark#1460Summary
rstudioapi::jobRunScript()now starts a new R console session in the background, and that session runs the script withsource().The caller's console stays selected and free while the script runs. For example,
targets::tar_make(as_job = TRUE)now runs the pipeline in its own console. Before this change, the call failed with "Function runScriptJob not found in RStudio".Ark sends a new UI comm request,
execute_code_in_new_session, with the code and a session name. Positron starts a new console session of the caller's runtime and queues the code in it. The new session does not become the active console or the foreground session, and Positron does not open the Console view. The request returns the new session ID after the code is in the queue, and the console info popup shows a new start reason:The R side uses
setwd()in the job code and notILanguageRuntimeSession.setWorkingDirectory(). I outlined why in posit-dev/ark#1460 but in short,setWorkingDirectory()for R does not wait for the result, it runs outside the console queue, and it does not change where.Rprofileand renv start.Known limitations
.Rprofileor renv.executeCode().Release Notes
New Features
rstudioapi::jobRunScript()now runs the script in a new R console in the background, so the current console stays free. As a result,targets::tar_make(as_job = TRUE)works (Multisession console: add support to programmatically launch new console sessions #6720).Bug Fixes
Validation Steps
@:console @:sessions @:plots @:ark
The unit tests cover the service changes:
To do a manual test, make
foo.RwithSys.sleep(10); print(getwd())andplot.Rwithplot(1:10). Then:rstudioapi::jobRunScript("foo.R"). After a little bit, the call returns. A new console namedfoo.Rshows in the session list, and the original console stays selected. You can type in the original console while the script runs. The new console prints the folder of the script.rstudioapi::jobRunScript("foo.R"); rstudioapi::jobRunScript("foo.R"). You get two new consoles.rstudioapi::jobRunScript("plot.R"). The plot shows in the Plots history, but the sidebar does not change its active tab, and focus stays in the console.rstudioapi::jobRunScript("foo.R"), and restart the original console while the new session starts. The original console restarts after the new session is ready.rstudioapi::jobRunScript("foo.R", workingDir = "no-such-dir"). You get an error in the original console, and no new console starts.targetsinstalled and a_targets.Rproject, runtargets::tar_make(as_job = TRUE). A new console shows the pipeline output, and the original console stays free.You can also use
.ps.ui.executeCodeInNewSession("plot(sample(100, 10))", "Howdy!")directly.