Activate conda environments prior to creating R kernel sessions #10471
Conversation
When R is installed in a conda/mamba environment, the conda environment must be activated to ensure compilation tools and dependencies are available in PATH. This change implements full conda environment activation when starting R sessions from conda-based installations. Key changes: - Added condaEnvironmentPath field to RMetadataExtra and RInstallation - Created conda-activation module to activate conda/mamba environments and capture environment variables - Updated kernel spec creation to merge conda environment variables - Added experimental positron.r.interpreters.condaDiscovery setting to UI - Supports both conda and mamba, with platform-specific activation The conda discovery setting is opt-in (default: false) and clearly marked as experimental in the settings UI. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
E2E Tests 🚀 |
| "type": "boolean", | ||
| "default": false, | ||
| "tags": [ | ||
| "interpreterSettings" |
There was a problem hiding this comment.
| "interpreterSettings" | |
| "experimental", | |
| "interpreterSettings" |
If we're going to do this, instead of using the word "experimental" in the description, let's use the tag that is used throughout the product.
jmcphers
left a comment
jmcphers
left a comment
There was a problem hiding this comment.
In general I'm a little worried about this approach for a couple of reasons:
- it's passing a large number of environment variables into the kernel, only a handful of which are the ones we are interested in (and we don't know which they are)
- there may be shell or session specific values that are captured in this temporary environment and then passed into the kernel where they may cause problems
- non-environment side effects from e.g. custom activate scripts won't be captured
- it relies on shell configuration and startup particulars which have proven to be a source of problems in the past (e.g. this doesn't handle
cshbut we learned pretty quickly after adding login shell support to the supervisor that some customer environments are only set up for csh)
I think the most robust approach would be to run conda activate in the same shell as the kernel itself, which would look something like this:
- the kernel supervisor's create method gets a new argument that accepts a preflight script to run before starting
- this preflight method implies the "run in shell" flag
- conda R sessions pass
conda activate fooas the preflight script - the kernel supervisor takes care of the rest
If you agree I'd be happy to take care of the kernel supervisor side of this approach.
| // Execute the command to get the environment | ||
| const { stdout } = await execPromise(command, { | ||
| maxBuffer: 10 * 1024 * 1024, // 10MB buffer for large environments | ||
| timeout: 30000 // 30 second timeout |
There was a problem hiding this comment.
If there's any chance that this is going to take > 2 seconds then we should show some kind of progress toast while we're doing it.
|
|
||
| // Construct path to activation script | ||
| let activationScript: string; | ||
| if (shellName.includes('fish')) { |
There was a problem hiding this comment.
We would need to handle csh/tcsh here too, and maybe even /bin/sh
Makes sense to me. The biggest question is whether we're aligned on trying to get this to work (it seems better than letting people use conda environments, but for them to be crippled in Positron) — if it isn't a hardship to implement and does not create ongoing maintenance burden, it seems like a reasonable thing to do to support the user community. |
|
I think any advance on the conda support front needs to be a more complete solution: specifically, preventing folks from shooting themselves in the foot and installing add-on packages built with an incompatible toolchain. R package devs have dealt with many, many issues over the years from users unwittingly creating dysfunctional situations. That solution would probably be something along the lines of disabling |
|
IMO some of the guardrails @jennybc outlines would be absolutely required if we ever turn on this discovery by default. With this behind an opt-in setting, I'm a bit more flexible as to whether we do that work right now. At a minimum, can we call out the problems with |
OK, we're going to need this for environment modules anyway (#8075) so I put this together in posit-dev/kallichore#55. When that's merged, you can pass |
Yeah, I think at minimum that plus something in the Positron documentation to indicate that users mix install.packages and |
|
Ideas for getting a sense of downstream consequences/gains/losses:
|
I had Claude Code take a look at the issues with activating conda environments prior to launching the R session related to #10359 and #4398. I don't see the harm in exposing the conda discovery setting in the UI as long as it is clearly marked experimental. There are some details in here (e.g. some methods that were made async) that would need to be reviewed more closely if there is alignment on the functionality. Feedback welcome.
(below is from claude code)
When R is installed in a conda/mamba environment, the conda environment must be activated to ensure compilation tools and dependencies are available in PATH. This change implements full conda environment activation when starting R sessions from conda-based installations.
Key changes:
The conda discovery setting is opt-in (default: false) and clearly marked as experimental in the settings UI.
Release Notes
New Features
Bug Fixes
QA Notes