Support optional python and r section attributes

[sagerb]

Intent

The changes within this PR support the initialization of config files to have default attributes associated with R and Python dependencies, allowing the sections to be empty within the config file. When values are present, they are used by the back end, but when they are not present, the current values from the active interpreters are used, as the deployment manifest is built from the config file.

Resolves #2468
Resolves #2470

Type of Change

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

Approach

To implement this functionality, it is important to have the config file written out without the attributes, but when it is passed around internally (and retrieved by the UX), that it have the current defaults inserted into the config object. Since the UX only patches the config file, it is important that the handling code really does "patch" rather than replace the object to disk (which would write out the defaults, which then become user-provided values).

To support the insertion of the defaults internally, some of the APIs needed to be updated to pass in the active interpreters. The insertion has been isolated into one specific spot; the point at which the code builds the config object from the file.

Since so many locations in the codebase are now using the interpreters, I've also modified the method in which they are initialized. Rather than pass around the executable string, the particular interpreter is built from that executable string, and then passed around to be used where needed. This is important to be done at the API level, so that when a user changes their selection or settings for the active interpreters are picked up at the next API call.

User Impact

New feature: The scenario goes like this:

1. A publisher has created a deployment that uses their currently active R and/or Python interpreters as the versions requested in the manifest.
2. The next day, they upgrade or change their active interpreters and then deploy again. These re-deployments once again request the active R and/or Python interpreters within the manifest.
3. If at any point the user sets the attribute values within the R or Python section, they will be used rather than floating to the active interpreter values.

Automated Tests

Unit tests updated.

Directions for Reviewers

Best to review this PR via commits. The list of commits have been updated to describe the changes.

• Update Schema to make attributes inside of R and Python be optional.
• Change to optionally populate R and Python sections during initial configuration initialization if told to. This will return new configs with empty sections (signaling to use defaults from interpreters), but still allow testing.
• extract active R and Python paths from API requests
• Omit R and Python version from config when empty
• Pass interpreters in versus creating interpreters within inspect
• add logging to know what is being passed up in manifest
• update initialization of publishing state to allow the loading of the config to fill in the missing pieces using the active interpreters (in config.FromFile)
• Update fromFile to fill in the empty fields into the config object, using the active interpreters
• update unit tests for agent
• update unused CLI command handlers just to compile...
• Utilize a TS class to make sure we pass in the proper interpreter (R vs. Python), since they are only strings by nature and order in the call functions can get very messed up.
• Update the UX API signatures to include R & Python

Checklist

• I have updated CHANGELOG.md to cover notable changes.

[dotNomad]

I'll note that we should definitely change the Configuration file reference document as part of this PR since this will make the Python settings and R settings sections out of date.

[sagerb]

I'll note that we should definitely change the Configuration file reference document as part of this PR since this will make the Python settings and R settings sections out of date.

I pushed up an update!

[marcosnav]

Could you elaborate more on why it is required to send an R and Python executables to get a configuration? Shouldn't the configuration response be what the existing file contains?

[dotNomad]

but when it is passed around internally (and retrieved by the UX), that it have the current defaults inserted into the config object.

As a part of keeping this PR as narrow as possible we didn't change the behavior in the API and frontend.

Currently in main the defaults are inserted into the response even if the file doesn't have those attributes set (like the packageFile = 'requirements.txt settings). Rather than changing both the backend and frontend here we kept the API behavior the same.

[marcosnav]

I think I understand the reason of having the resulting config in the UI to have some defaults, but it feels confusing the way it is being handled.

This method is one of many that now accepts R and Python executables, but feels out of place, for example the getPythonPackages calling configurations/{name}/packages/python also accepts an R executable, and I find it confusing for future folks to have a method responsible to get python packages to accept an R executable.

The go method FromFile now accepts overriding defaults as activeRInterpreter and activePythonInterpreter, and we are now having a method that at first look suggests to load the configuration as it is on file, but consumers of it could find surprises from it.

My suggestion would be, to take a step back a bit and think how these signatures bring clarity or confusion to future folks working on this, specifically from the point that future folks might not have all the context on how this works or the history behind this.

[dotNomad]

Great point. The effort here was to avoid making changes on the frontend, but creating confusion by needing to pass in the Python and R executables everywhere is confusing.

[sagerb]

The go method FromFile now accepts overriding defaults as activeRInterpreter and activePythonInterpreter

To clarify, it's not overriding defaults but actually implementing them. The defaults come from the active interpreters, passed in at the time of the API request. Our current architecture requires all the levels of calls to pass these down into the deepest functions that need it. Unfortunately, that is very deep in this case.

I understand about the confusion. Let me think about this, and I'll signal for another review. Thanks.

[sagerb]

I've thought about the approach used in this PR and I've concluded that we introduced confusion because we were attempting to not modify the extension UX.

I'm going to update the related issue with our learnings, so that we can implement something else here.

I'll be closing this PR without merging.