Skip to content

fix(runtime): Fix inconsistent runtime discovery and selection when deploying from Positron #2708

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

Merged
merged 4 commits into from
Jul 18, 2025

Conversation

marcosnav
Copy link
Collaborator

@marcosnav marcosnav commented Jul 16, 2025

Intent

Fix inconsistent runtime discovery and selection when deploying from Positron.

Fixes #2648 #2663

Type of Change

    • Bug Fix
    • New Feature
    • Breaking Change
    • Documentation
    • Refactor
    • Tooling

Approach

The API deployment handler was taking into account the preferred runtime at the beginning but it was ignoring it after some initial validations.

Updated the necessary methods to make use of the preferred runtime across the complete deployment request.

User Impact

Users should be able to deploy with the selected runtime in Positron.

Automated Tests

Updated the related unit tests

Directions for Reviewers

Try deploying content from Positron, while selecting a different R or Python version and seeing the deployment go through with success.

Checklist

@marcosnav marcosnav changed the title fix(runtime): Fix inconsistent runtime discovery and selection when d… fix(runtime): Fix inconsistent runtime discovery and selection when deploying Jul 17, 2025
@marcosnav marcosnav changed the title fix(runtime): Fix inconsistent runtime discovery and selection when deploying fix(runtime): Fix inconsistent runtime discovery and selection when deploying from Positron Jul 17, 2025
@marcosnav marcosnav marked this pull request as ready for review July 17, 2025 15:59
@marcosnav marcosnav requested a review from tdstein July 17, 2025 16:00
Copy link
Collaborator

@dotNomad dotNomad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pulled this down and tried this out in Positron using the VSIX from the CI dist artifact.

I saw the same issue described. When publishing with a non-default R version installed I see the output:

ime=2025-07-17T11:04:10.487-07:00 level=INFO msg="Getting R version" r=/usr/local/bin/R
time=2025-07-17T11:04:10.487-07:00 level=DEBUG msg="Running command" cmd=/usr/local/bin/R args=--version
time=2025-07-17T11:04:10.522-07:00 level=INFO msg="Parsing line for R version" l="R version 4.4.2 (2024-10-31) -- \"Pile of Leaves\""
time=2025-07-17T11:04:10.522-07:00 level=INFO msg="Detected R version" version=4.4.2
ime=2025-07-17T11:04:10.522-07:00 level=DEBUG msg="Successful validation for R executable" rExecutable=/usr/local/bin/R
time=2025-07-17T11:04:10.522-07:00 level=INFO msg="Getting renv lockfile path from R executable" r=/usr/local/bin/R
time=2025-07-17T11:04:10.522-07:00 level=DEBUG msg="Running script" cmd=/usr/local/bin/R args=-s script=renv::paths$lockfile()
time=2025-07-17T11:04:10.529-07:00 level=DEBUG msg="Running command" cmd=/usr/local/bin/R args="-s -f /var/folders/wn/83k9zr2s791brfmfk90c_6vh0000gp/T/3474042576"
time=2025-07-17T11:04:11.179-07:00 level=INFO msg="Parsing line for renv::path output" output="- None of the packages recorded in the lockfile are currently installed."
time=2025-07-17T11:04:11.179-07:00 level=INFO msg="Parsing line for renv::path output" output="- Use `renv::restore()` to restore the project library."

and the error notification

Image

I was able to publish just fine using the default R interpreter on PATH, but when using the non-default I had the problems above. Let me describe a bit of how I got there.

I use rig to install multiple versions of R.

❯ rig list
* name       version    aliases
------------------------------------------
  4.3-arm64  (R 4.3.3)
* 4.4-arm64  (R 4.4.2)  release

When using Positron with that R 4.4.2 install everything works great since that is the R that I'm pointed to on PATH.

When I choose the 4.3.3 interpreter in Positron is when I start to see those errors. 4.3.3 is detected in a ton of spots in the Posit Publisher Deployment output, but as soon as I get to the renv checking it wants to use the R on PATH.

Happy to provide more information on how to reproduce this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can leave of VS Code changelog item changes since that usually part of the release flow (to copy over the extension specific changelog items)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still lingering, but its totally fine to leave in since the release flow will update it anyway

@marcosnav
Copy link
Collaborator Author

@dotNomad Thank you for trying it out. I pushed a new commit addressing the behavior you are seeing. In more detail, the runtime mismatch was happening in two steps:

  1. Deploying an existing deployment, there is a two step runtime check that was inconsistent, this one got fixed when creating the PR.
  2. Creating a brand new deployment, inspection returns possible configurations, such inspection was using inconsistent runtime versions, the issue you found and now fixed with the latest commit.

@marcosnav marcosnav requested a review from dotNomad July 18, 2025 18:10
Comment on lines -21 to -24
type postInspectRequestBody struct {
Python string `json:"python"`
R string `json:"r"`
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wasn't used anymore, removed it

Comment on lines +62 to +68
type postInspectHandler struct {
base util.AbsolutePath
log logging.Logger
initializer configGetter
matchingWalker func([]string, util.AbsolutePath, logging.Logger) (util.Walker, error)
interpretersResolve func(util.AbsolutePath, http.ResponseWriter, *http.Request, logging.Logger) (interpreters.RInterpreter, interpreters.PythonInterpreter, error)
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the bump on changes, I had to re-wire a bit the inspect endpoint handler in order to be testable. We didn't had tests on this one so I went for it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries thanks for adding the additional tests!

return err
}
// Parse and resolve executable paths for Python and R
rInterpreter, pythonInterpreter, err := h.interpretersResolve(projectDir, w, req, h.log)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the most important part when it comes to solving the bug. Account for the preferred interpreters coming in the request and use those throughout inspection operations.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for highlighting this - it makes it much easier to review

Copy link
Collaborator

@dotNomad dotNomad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pulled this down and tried this on Positron. I attempted a deploy using the default and non-default R interpreter I have installed using rig - worked great. I also changed the interpreter I had selected, updated the renv.lock and was able to push an update, again worked perfectly.

Absolutely fantastic work @marcosnav

Comment on lines +62 to +68
type postInspectHandler struct {
base util.AbsolutePath
log logging.Logger
initializer configGetter
matchingWalker func([]string, util.AbsolutePath, logging.Logger) (util.Walker, error)
interpretersResolve func(util.AbsolutePath, http.ResponseWriter, *http.Request, logging.Logger) (interpreters.RInterpreter, interpreters.PythonInterpreter, error)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries thanks for adding the additional tests!

return err
}
// Parse and resolve executable paths for Python and R
rInterpreter, pythonInterpreter, err := h.interpretersResolve(projectDir, w, req, h.log)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for highlighting this - it makes it much easier to review

@marcosnav marcosnav merged commit f3df906 into main Jul 18, 2025
12 checks passed
@marcosnav marcosnav deleted the mnv-fix-deployment-runtime-mismatch branch July 18, 2025 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Not detecting the R interpreter that is active in positron
2 participants