Skip to content
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

Update manifest.json R version population #2468

Open
m-- opened this issue Dec 6, 2024 · 0 comments
Open

Update manifest.json R version population #2468

m-- opened this issue Dec 6, 2024 · 0 comments
Assignees
Milestone

Comments

@m--
Copy link
Collaborator

m-- commented Dec 6, 2024

We should update how we decide which R version we specify for running on Connect. This work would update the the R.version field in the config to be optional, and document what it means to set it. This should also update how we build the manifest.json accordingly:

  1. From the R.version in the config, if set (this field is now optional)
  2. From the current active version of R in the IDE, if the R.version is not set in config

The answer to this should populate the platform field in the manifest.json, when we build it.

Note that this doesn't lean on the R version defined in renv.lock, which, ultimately, we may want to do. If we tackle that change, it will be in a follow-on issue.


After analyzing feedback from the original PR that was created to address this issue (#2568), we've decided that the alternative design which was informally discussed would be a better direction to go in.

With the implementation of default values, for R and Python, derived from the active interpreters at the time of deployment, the challenge has been how to keep the concept of a default wanted vs. a locked-in (supplied) value within the extension UX, which depends on values for these fields.

The attributes / responsibility of this alternative design would involve maintaining the internal representation of the configuration objects as they are represented on disk, with the addition of default value fields. The deployment process (in the agent) would be responsible for resolving the defaults, at the time needed when generating the maniftest. The extension UX would be responsible for converting the configuration objects queried from the agent w/ the active "expected" default values.

Current thoughts on this path: the configuration object can can be extended to include default values inserted during transit. This would maintain being able to identify the difference, but would make the users of this field responsible for looking possibly at both values. (However, this can be done within the existing store, with an explicit computed value.)

@sagerb sagerb self-assigned this Dec 6, 2024
@sagerb sagerb added this to the v1.6.0 milestone Dec 6, 2024
@sagerb sagerb changed the title Update manifest.json R version (platform) population Update manifest.json R version population Dec 9, 2024
@dotNomad dotNomad mentioned this issue Dec 9, 2024
@sagerb sagerb mentioned this issue Dec 10, 2024
7 tasks
@m-- m-- modified the milestones: v1.6.0, v.1.8.0 Dec 10, 2024
@dotNomad dotNomad modified the milestones: v1.8.0, v1.10.0 Jan 7, 2025
@dotNomad dotNomad modified the milestones: v1.10.0, v1.12.0 Jan 29, 2025
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 a pull request may close this issue.

3 participants