From 5e707c01e2acd6dcfc5977362bd3116476eb79be Mon Sep 17 00:00:00 2001 From: Michael Marchetti Date: Thu, 15 Aug 2024 11:39:43 -0400 Subject: [PATCH 01/11] Scan and deploy APIs accept an R parameter --- cmd/publisher/commands/deploy.go | 4 +++- cmd/publisher/commands/redeploy.go | 3 ++- internal/publish/publish.go | 4 ++-- internal/publish/publish_test.go | 2 +- internal/services/api/patch_deployment_test.go | 5 ----- internal/services/api/post_deployment.go | 4 +++- internal/services/api/post_deployment_test.go | 6 +++--- internal/services/api/post_inspect.go | 5 ++++- internal/services/api/post_packages_r_scan.go | 4 +++- internal/services/api/post_packages_r_scan_test.go | 7 +++++-- 10 files changed, 26 insertions(+), 18 deletions(-) diff --git a/cmd/publisher/commands/deploy.go b/cmd/publisher/commands/deploy.go index 9535049e4..c467e79be 100644 --- a/cmd/publisher/commands/deploy.go +++ b/cmd/publisher/commands/deploy.go @@ -65,7 +65,9 @@ func (cmd *DeployCmd) Run(args *cli_types.CommonArgs, ctx *cli_types.CLIContext) stateStore.Account.Name, stateStore.ConfigName, stateStore.SaveName) - publisher, err := publish.NewFromState(stateStore, events.NewCliEmitter(os.Stderr, ctx.Logger), ctx.Logger) + + rExecutable := util.Path{} + publisher, err := publish.NewFromState(stateStore, rExecutable, events.NewCliEmitter(os.Stderr, ctx.Logger), ctx.Logger) if err != nil { return err } diff --git a/cmd/publisher/commands/redeploy.go b/cmd/publisher/commands/redeploy.go index ed4924201..d5206d122 100644 --- a/cmd/publisher/commands/redeploy.go +++ b/cmd/publisher/commands/redeploy.go @@ -51,7 +51,8 @@ func (cmd *RedeployCmd) Run(args *cli_types.CommonArgs, ctx *cli_types.CLIContex stateStore.Account.Name, stateStore.ConfigName) - publisher, err := publish.NewFromState(stateStore, events.NewCliEmitter(os.Stderr, ctx.Logger), ctx.Logger) + rExecutable := util.Path{} + publisher, err := publish.NewFromState(stateStore, rExecutable, events.NewCliEmitter(os.Stderr, ctx.Logger), ctx.Logger) if err != nil { return err } diff --git a/internal/publish/publish.go b/internal/publish/publish.go index a6ad681fc..a0d3c18db 100644 --- a/internal/publish/publish.go +++ b/internal/publish/publish.go @@ -62,7 +62,7 @@ type publishDeployedFailureData struct { DirectURL string `mapstructure:"url"` } -func NewFromState(s *state.State, emitter events.Emitter, log logging.Logger) (Publisher, error) { +func NewFromState(s *state.State, rExecutable util.Path, emitter events.Emitter, log logging.Logger) (Publisher, error) { if s.LocalID != "" { data := baseEventData{ LocalID: s.LocalID, @@ -77,7 +77,7 @@ func NewFromState(s *state.State, emitter events.Emitter, log logging.Logger) (P return &defaultPublisher{ State: s, emitter: emitter, - rPackageMapper: renv.NewPackageMapper(s.Dir, util.Path{}), + rPackageMapper: renv.NewPackageMapper(s.Dir, rExecutable), }, nil } diff --git a/internal/publish/publish_test.go b/internal/publish/publish_test.go index bff517f09..9bb80bff7 100644 --- a/internal/publish/publish_test.go +++ b/internal/publish/publish_test.go @@ -102,7 +102,7 @@ func (s *PublishSuite) SetupTest() { func (s *PublishSuite) TestNewFromState() { stateStore := state.Empty() - publisher, err := NewFromState(stateStore, events.NewNullEmitter(), logging.New()) + publisher, err := NewFromState(stateStore, util.Path{}, events.NewNullEmitter(), logging.New()) s.NoError(err) s.Equal(stateStore, publisher.(*defaultPublisher).State) } diff --git a/internal/services/api/patch_deployment_test.go b/internal/services/api/patch_deployment_test.go index 21f9b46e2..255ddcffc 100644 --- a/internal/services/api/patch_deployment_test.go +++ b/internal/services/api/patch_deployment_test.go @@ -14,8 +14,6 @@ import ( "github.com/posit-dev/publisher/internal/config" "github.com/posit-dev/publisher/internal/deployment" "github.com/posit-dev/publisher/internal/logging" - "github.com/posit-dev/publisher/internal/publish" - "github.com/posit-dev/publisher/internal/state" "github.com/posit-dev/publisher/internal/util" "github.com/posit-dev/publisher/internal/util/utiltest" "github.com/spf13/afero" @@ -32,9 +30,6 @@ func TestPatchDeploymentHandlerFuncSuite(t *testing.T) { } func (s *PatchDeploymentHandlerFuncSuite) SetupTest() { - stateFactory = state.New - publisherFactory = publish.NewFromState - afs := afero.NewMemMapFs() cwd, err := util.Getwd(afs) s.Nil(err) diff --git a/internal/services/api/post_deployment.go b/internal/services/api/post_deployment.go index c4d0d7d31..722d2f321 100644 --- a/internal/services/api/post_deployment.go +++ b/internal/services/api/post_deployment.go @@ -19,6 +19,7 @@ import ( type PostDeploymentRequestBody struct { AccountName string `json:"account"` ConfigName string `json:"config"` + R string `json:"r"` } type PostDeploymentsReponse struct { @@ -77,7 +78,8 @@ func PostDeploymentHandlerFunc( json.NewEncoder(w).Encode(response) newState.LocalID = localID - publisher, err := publisherFactory(newState, emitter, log) + rExecutable := util.NewPath(b.R, nil) + publisher, err := publisherFactory(newState, rExecutable, emitter, log) if err != nil { InternalError(w, req, log, err) return diff --git a/internal/services/api/post_deployment_test.go b/internal/services/api/post_deployment_test.go index 3b77547ed..069ff9d7d 100644 --- a/internal/services/api/post_deployment_test.go +++ b/internal/services/api/post_deployment_test.go @@ -72,7 +72,7 @@ func (s *PostDeploymentHandlerFuncSuite) TestPostDeploymentHandlerFunc() { publisher := &mockPublisher{} publisher.On("PublishDirectory", mock.Anything).Return(nil) - publisherFactory = func(*state.State, events.Emitter, logging.Logger) (publish.Publisher, error) { + publisherFactory = func(*state.State, util.Path, events.Emitter, logging.Logger) (publish.Publisher, error) { return publisher, nil } stateFactory = func( @@ -194,7 +194,7 @@ func (s *PostDeploymentHandlerFuncSuite) TestPostDeploymentHandlerFuncPublishErr testErr := errors.New("test error from PublishDirectory") publisher := &mockPublisher{} publisher.On("PublishDirectory", mock.Anything).Return(testErr) - publisherFactory = func(*state.State, events.Emitter, logging.Logger) (publish.Publisher, error) { + publisherFactory = func(*state.State, util.Path, events.Emitter, logging.Logger) (publish.Publisher, error) { return publisher, nil } @@ -229,7 +229,7 @@ func (s *PostDeploymentHandlerFuncSuite) TestPostDeploymentSubdir() { publisher := &mockPublisher{} publisher.On("PublishDirectory", mock.Anything).Return(nil) - publisherFactory = func(*state.State, events.Emitter, logging.Logger) (publish.Publisher, error) { + publisherFactory = func(*state.State, util.Path, events.Emitter, logging.Logger) (publish.Publisher, error) { return publisher, nil } stateFactory = func( diff --git a/internal/services/api/post_inspect.go b/internal/services/api/post_inspect.go index f891d5fc3..b0bcf8ce8 100644 --- a/internal/services/api/post_inspect.go +++ b/internal/services/api/post_inspect.go @@ -19,6 +19,7 @@ import ( type postInspectRequestBody struct { Python string `json:"python"` + R string `json:"r"` } type postInspectResponseBody struct { @@ -73,6 +74,8 @@ func PostInspectHandlerFunc(base util.AbsolutePath, log logging.Logger) http.Han return } pythonPath := util.NewPath(b.Python, nil) + rPath := util.NewPath(b.R, nil) + response := []postInspectResponseBody{} if req.URL.Query().Get("recursive") == "true" { @@ -102,7 +105,7 @@ func PostInspectHandlerFunc(base util.AbsolutePath, log logging.Logger) http.Han } entrypoint := req.URL.Query().Get("entrypoint") entrypointPath := util.NewRelativePath(entrypoint, base.Fs()) - configs, err := initialize.GetPossibleConfigs(path, pythonPath, util.Path{}, entrypointPath, log) + configs, err := initialize.GetPossibleConfigs(path, pythonPath, rPath, entrypointPath, log) if err != nil { return err } diff --git a/internal/services/api/post_packages_r_scan.go b/internal/services/api/post_packages_r_scan.go index 9bf847074..8f7b536e0 100644 --- a/internal/services/api/post_packages_r_scan.go +++ b/internal/services/api/post_packages_r_scan.go @@ -15,6 +15,7 @@ import ( ) type PostPackagesRScanRequest struct { + R string `json:"r"` SaveName string `json:"saveName"` } @@ -58,7 +59,8 @@ func (h *PostPackagesRScanHandler) ServeHTTP(w http.ResponseWriter, req *http.Re return } lockfileAbsPath := projectDir.Join(path.String()) - inspector := rInspectorFactory(projectDir, util.Path{}, h.log) + rPath := util.NewPath(b.R, nil) + inspector := rInspectorFactory(projectDir, rPath, h.log) err = inspector.CreateLockfile(lockfileAbsPath) if err != nil { InternalError(w, req, h.log, err) diff --git a/internal/services/api/post_packages_r_scan_test.go b/internal/services/api/post_packages_r_scan_test.go index 4a1692e14..dccebe20e 100644 --- a/internal/services/api/post_packages_r_scan_test.go +++ b/internal/services/api/post_packages_r_scan_test.go @@ -40,7 +40,7 @@ func (s *PostPackagesRScanSuite) TestNewPostPackagesRScanHandler() { func (s *PostPackagesRScanSuite) TestServeHTTP() { rec := httptest.NewRecorder() - body := strings.NewReader(`{"saveName":""}`) + body := strings.NewReader(`{"saveName":"", "r": "/opt/R/bin/R"}`) req, err := http.NewRequest("POST", "/api/packages/r/scan", body) s.NoError(err) @@ -52,7 +52,10 @@ func (s *PostPackagesRScanSuite) TestServeHTTP() { log := logging.New() h := NewPostPackagesRScanHandler(base, log) - rInspectorFactory = func(util.AbsolutePath, util.Path, logging.Logger) inspect.RInspector { + rInspectorFactory = func(baseDir util.AbsolutePath, rExec util.Path, log logging.Logger) inspect.RInspector { + s.Equal(base, baseDir) + s.Equal(util.NewPath("/opt/R/bin/R", nil), rExec) + i := inspect.NewMockRInspector() i.On("CreateLockfile", destPath).Return(nil) return i From 96f0276aa558d0b0a6c90bd8123989c18482240d Mon Sep 17 00:00:00 2001 From: Michael Marchetti Date: Thu, 15 Aug 2024 12:45:24 -0400 Subject: [PATCH 02/11] Get the current R interpreter from Positron --- extensions/vscode/src/@types/positron.d.ts | 74 +++++++++++++++++++ .../src/api/resources/Configurations.ts | 2 + extensions/vscode/src/entrypointTracker.ts | 8 +- .../src/multiStepInputs/newDeployment.ts | 17 ++++- .../selectNewOrExistingConfig.ts | 8 +- extensions/vscode/src/utils/config.ts | 32 ++++++++ 6 files changed, 134 insertions(+), 7 deletions(-) create mode 100644 extensions/vscode/src/@types/positron.d.ts diff --git a/extensions/vscode/src/@types/positron.d.ts b/extensions/vscode/src/@types/positron.d.ts new file mode 100644 index 000000000..5b4c640f6 --- /dev/null +++ b/extensions/vscode/src/@types/positron.d.ts @@ -0,0 +1,74 @@ +// Copyright (C) 2024 by Posit Software, PBC. + +// This is the portion of the Positron API definition +// used by Publisher, here until it is published. + +declare module "positron" { + export interface PositronApi { + version: string; + runtime: runtime; + } + + /** + * LanguageRuntimeMetadata contains information about a language runtime that is known + * before the runtime is started. + */ + export interface LanguageRuntimeMetadata { + /** The path to the runtime. */ + runtimePath: string; + + /** A unique identifier for this runtime; takes the form of a GUID */ + runtimeId: string; + + /** + * The fully qualified name of the runtime displayed to the user; e.g. "R 4.2 (64-bit)". + * Should be unique across languages. + */ + runtimeName: string; + + /** + * A language specific runtime name displayed to the user; e.g. "4.2 (64-bit)". + * Should be unique within a single language. + */ + runtimeShortName: string; + + /** The version of the runtime itself (e.g. kernel or extension version) as a string; e.g. "0.1" */ + runtimeVersion: string; + + /** The runtime's source or origin; e.g. PyEnv, System, Homebrew, Conda, etc. */ + runtimeSource: string; + + /** The free-form, user-friendly name of the language this runtime can execute; e.g. "R" */ + languageName: string; + + /** + * The Visual Studio Code Language ID of the language this runtime can execute; e.g. "r" + * + * See here for a list of known language IDs: + * https://code.visualstudio.com/docs/languages/identifiers#_known-language-identifiers + */ + languageId: string; + + /** The version of the language; e.g. "4.2" */ + languageVersion: string; + + /** The Base64-encoded icon SVG for the language. */ + base64EncodedIconSvg: string | undefined; + + /** Whether the runtime should start up automatically or wait until explicitly requested */ + // startupBehavior: LanguageRuntimeStartupBehavior; + + /** Where sessions will be located; used as a hint to control session restoration */ + // sessionLocation: LanguageRuntimeSessionLocation; + + /** + * Extra data supplied by the runtime provider; not read by Positron but supplied + * when creating a new session from the metadata. + */ + extraRuntimeData: any; + } + + export interface runtime { + getPreferredRuntime(languageId: string): Thenable; + } +} diff --git a/extensions/vscode/src/api/resources/Configurations.ts b/extensions/vscode/src/api/resources/Configurations.ts index 151df4789..a1f7176bf 100644 --- a/extensions/vscode/src/api/resources/Configurations.ts +++ b/extensions/vscode/src/api/resources/Configurations.ts @@ -81,12 +81,14 @@ export class Configurations { inspect( dir: string, python?: string, + r?: string, params?: { entrypoint?: string; recursive?: boolean }, ) { return this.client.post( "/inspect", { python, + r, }, { params: { diff --git a/extensions/vscode/src/entrypointTracker.ts b/extensions/vscode/src/entrypointTracker.ts index bc3c56e18..1b480bdd9 100644 --- a/extensions/vscode/src/entrypointTracker.ts +++ b/extensions/vscode/src/entrypointTracker.ts @@ -14,7 +14,10 @@ import { Utils as uriUtils } from "vscode-uri"; import { useApi } from "src/api"; import { Contexts } from "src/constants"; -import { getPythonInterpreterPath } from "src/utils/config"; +import { + getPythonInterpreterPath, + getRInterpreterPath, +} from "src/utils/config"; import { isActiveDocument, relativeDir } from "src/utils/files"; import { hasKnownContentType } from "src/utils/inspect"; import { getSummaryStringFromError } from "src/utils/errors"; @@ -43,8 +46,9 @@ async function isDocumentEntrypoint( try { const api = await useApi(); const python = await getPythonInterpreterPath(); + const r = await getRInterpreterPath(); - const response = await api.configurations.inspect(dir, python, { + const response = await api.configurations.inspect(dir, python, r, { entrypoint: uriUtils.basename(document.uri), }); diff --git a/extensions/vscode/src/multiStepInputs/newDeployment.ts b/extensions/vscode/src/multiStepInputs/newDeployment.ts index bd91316fd..9efcc0836 100644 --- a/extensions/vscode/src/multiStepInputs/newDeployment.ts +++ b/extensions/vscode/src/multiStepInputs/newDeployment.ts @@ -28,7 +28,10 @@ import { ConfigurationInspectionResult, EntryPointPath, } from "src/api"; -import { getPythonInterpreterPath } from "src/utils/config"; +import { + getPythonInterpreterPath, + getRInterpreterPath, +} from "src/utils/config"; import { getMessageFromError, getSummaryStringFromError, @@ -528,12 +531,18 @@ export async function newDeployment( try { const python = await getPythonInterpreterPath(); + const r = await getRInterpreterPath(); const dir = path.dirname(relEntryPoint); const entryPointFile = path.basename(relEntryPoint); - const inspectResponse = await api.configurations.inspect(dir, python, { - entrypoint: entryPointFile, - }); + const inspectResponse = await api.configurations.inspect( + dir, + python, + r, + { + entrypoint: entryPointFile, + }, + ); inspectionResults = inspectResponse.data; inspectionResults.forEach((result, i) => { diff --git a/extensions/vscode/src/multiStepInputs/selectNewOrExistingConfig.ts b/extensions/vscode/src/multiStepInputs/selectNewOrExistingConfig.ts index 85a03d55b..49373eefa 100644 --- a/extensions/vscode/src/multiStepInputs/selectNewOrExistingConfig.ts +++ b/extensions/vscode/src/multiStepInputs/selectNewOrExistingConfig.ts @@ -23,7 +23,10 @@ import { isConfigurationError, useApi, } from "src/api"; -import { getPythonInterpreterPath } from "src/utils/config"; +import { + getPythonInterpreterPath, + getRInterpreterPath, +} from "src/utils/config"; import { getSummaryStringFromError } from "src/utils/errors"; import { MultiStepInput, @@ -175,9 +178,12 @@ export async function selectNewOrExistingConfig( async (resolve, reject) => { try { const python = await getPythonInterpreterPath(); + const r = await getRInterpreterPath(); + const inspectResponse = await api.configurations.inspect( activeDeployment.projectDir, python, + r, ); inspectionResults = filterInspectionResultsToType( inspectResponse.data, diff --git a/extensions/vscode/src/utils/config.ts b/extensions/vscode/src/utils/config.ts index bb7cfcd69..3224b9d3a 100644 --- a/extensions/vscode/src/utils/config.ts +++ b/extensions/vscode/src/utils/config.ts @@ -3,6 +3,7 @@ import { Uri, commands, workspace } from "vscode"; import { fileExists, isDir } from "./files"; import { substituteVariables } from "./variables"; +import { PositronApi } from "positron"; export async function getPythonInterpreterPath(): Promise { const workspaceFolder = workspace.workspaceFolders?.[0]; @@ -40,3 +41,34 @@ export async function getPythonInterpreterPath(): Promise { console.log("Python interpreter path:", python); return python; } + +declare global { + function acquirePositronApi(): PositronApi; +} + +let positronApi: PositronApi | null | undefined; + +function getPositronApi(): PositronApi | null { + if (positronApi === undefined) { + try { + positronApi = acquirePositronApi(); + } catch { + positronApi = null; + } + } + return positronApi; +} + +export async function getRInterpreterPath(): Promise { + const api = getPositronApi(); + + if (api) { + const runtime = await api.runtime.getPreferredRuntime("r"); + if (runtime) { + return runtime.runtimePath; + } + } + // We don't know the interpreter path. + // The backend will run R from PATH. + return undefined; +} From 59b09a0926f897ef84629227fbe5ec1eabd315b3 Mon Sep 17 00:00:00 2001 From: Michael Marchetti Date: Fri, 16 Aug 2024 13:36:36 -0400 Subject: [PATCH 03/11] extension passes R to scan and deploy APIs --- extensions/vscode/src/api/resources/ContentRecords.ts | 2 ++ extensions/vscode/src/api/resources/Packages.ts | 4 ++-- extensions/vscode/src/utils/config.ts | 3 ++- extensions/vscode/src/views/homeView.ts | 8 +++++++- 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/extensions/vscode/src/api/resources/ContentRecords.ts b/extensions/vscode/src/api/resources/ContentRecords.ts index 5a564e82e..7e67b98e4 100644 --- a/extensions/vscode/src/api/resources/ContentRecords.ts +++ b/extensions/vscode/src/api/resources/ContentRecords.ts @@ -72,10 +72,12 @@ export class ContentRecords { accountName: string, configName: string, dir: string, + r?: string, ) { const data = { account: accountName, config: configName, + r: r, }; const encodedTarget = encodeURIComponent(targetName); return this.client.post<{ localId: string }>( diff --git a/extensions/vscode/src/api/resources/Packages.ts b/extensions/vscode/src/api/resources/Packages.ts index 4b6512c21..3429df19f 100644 --- a/extensions/vscode/src/api/resources/Packages.ts +++ b/extensions/vscode/src/api/resources/Packages.ts @@ -61,10 +61,10 @@ export class Packages { // 200 - success // 400 - bad request // 500 - internal server error - createRRequirementsFile(dir: string, saveName?: string) { + createRRequirementsFile(dir: string, saveName?: string, r?: string) { return this.client.post( "packages/r/scan", - { saveName }, + { saveName, r }, { params: { dir } }, ); } diff --git a/extensions/vscode/src/utils/config.ts b/extensions/vscode/src/utils/config.ts index 3224b9d3a..b022315ad 100644 --- a/extensions/vscode/src/utils/config.ts +++ b/extensions/vscode/src/utils/config.ts @@ -65,7 +65,8 @@ export async function getRInterpreterPath(): Promise { if (api) { const runtime = await api.runtime.getPreferredRuntime("r"); if (runtime) { - return runtime.runtimePath; + const interpreter = runtime.runtimePath; + return interpreter; } } // We don't know the interpreter path. diff --git a/extensions/vscode/src/views/homeView.ts b/extensions/vscode/src/views/homeView.ts index 829e43cdb..3302acf15 100644 --- a/extensions/vscode/src/views/homeView.ts +++ b/extensions/vscode/src/views/homeView.ts @@ -37,7 +37,7 @@ import { } from "src/api"; import { useBus } from "src/bus"; import { EventStream } from "src/events"; -import { getPythonInterpreterPath } from "../utils/config"; +import { getPythonInterpreterPath, getRInterpreterPath } from "../utils/config"; import { getSummaryStringFromError } from "src/utils/errors"; import { getNonce } from "src/utils/getNonce"; import { getUri } from "src/utils/getUri"; @@ -244,11 +244,14 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable { ) { try { const api = await useApi(); + const r = await getRInterpreterPath(); + const response = await api.contentRecords.publish( deploymentName, credentialName, configurationName, projectDir, + r, ); deployProject(response.data.localId, this.stream); } catch (error: unknown) { @@ -710,9 +713,12 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable { try { const api = await useApi(); + const r = await getRInterpreterPath(); + const apiRequest = api.packages.createRRequirementsFile( activeConfiguration.projectDir, relPathPackageFile, + r, ); showProgress("Creating R Requirements File", apiRequest, Views.HomeView); From 370710e4242c727e0152f0afd6ff9f4106f9d6a2 Mon Sep 17 00:00:00 2001 From: Michael Marchetti Date: Fri, 23 Aug 2024 10:32:35 -0400 Subject: [PATCH 04/11] rename utils/config.ts --- extensions/vscode/src/utils/{config.ts => vscode.ts} | 0 extensions/vscode/src/views/homeView.ts | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename extensions/vscode/src/utils/{config.ts => vscode.ts} (100%) diff --git a/extensions/vscode/src/utils/config.ts b/extensions/vscode/src/utils/vscode.ts similarity index 100% rename from extensions/vscode/src/utils/config.ts rename to extensions/vscode/src/utils/vscode.ts diff --git a/extensions/vscode/src/views/homeView.ts b/extensions/vscode/src/views/homeView.ts index 4b077eb06..e43f2148c 100644 --- a/extensions/vscode/src/views/homeView.ts +++ b/extensions/vscode/src/views/homeView.ts @@ -38,7 +38,7 @@ import { } from "src/api"; import { useBus } from "src/bus"; import { EventStream } from "src/events"; -import { getPythonInterpreterPath, getRInterpreterPath } from "../utils/config"; +import { getPythonInterpreterPath, getRInterpreterPath } from "../utils/vscode"; import { getSummaryStringFromError } from "src/utils/errors"; import { getNonce } from "src/utils/getNonce"; import { getUri } from "src/utils/getUri"; From f648ee8637e395cd4ab931cd1871e9198b9f086c Mon Sep 17 00:00:00 2001 From: Michael Marchetti Date: Fri, 23 Aug 2024 12:54:17 -0400 Subject: [PATCH 05/11] add logging to R interpreter detection --- extensions/vscode/src/utils/vscode.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/extensions/vscode/src/utils/vscode.ts b/extensions/vscode/src/utils/vscode.ts index b022315ad..126d74ae1 100644 --- a/extensions/vscode/src/utils/vscode.ts +++ b/extensions/vscode/src/utils/vscode.ts @@ -66,10 +66,18 @@ export async function getRInterpreterPath(): Promise { const runtime = await api.runtime.getPreferredRuntime("r"); if (runtime) { const interpreter = runtime.runtimePath; + console.log("Using selected R interpreter", interpreter); return interpreter; + } else { + console.log( + "Using default R interpreter because getPreferredRuntime did not return one", + ); } } // We don't know the interpreter path. // The backend will run R from PATH. + console.log( + "Using default R interpreter because the Positron API is not available", + ); return undefined; } From 9948fba05435c5ff4f2ca169f9b6edf32393fc47 Mon Sep 17 00:00:00 2001 From: Michael Marchetti Date: Fri, 23 Aug 2024 13:21:40 -0400 Subject: [PATCH 06/11] retry getPreferredRuntime --- extensions/vscode/src/utils/throttle.ts | 4 ++++ extensions/vscode/src/utils/vscode.ts | 25 +++++++++++++++++++++++-- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/extensions/vscode/src/utils/throttle.ts b/extensions/vscode/src/utils/throttle.ts index a7f61821c..46acb8a33 100644 --- a/extensions/vscode/src/utils/throttle.ts +++ b/extensions/vscode/src/utils/throttle.ts @@ -40,3 +40,7 @@ export const throttleWithLastPending = async ( } } }; + +export function delay(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} diff --git a/extensions/vscode/src/utils/vscode.ts b/extensions/vscode/src/utils/vscode.ts index 126d74ae1..2e620c033 100644 --- a/extensions/vscode/src/utils/vscode.ts +++ b/extensions/vscode/src/utils/vscode.ts @@ -2,8 +2,9 @@ import { Uri, commands, workspace } from "vscode"; import { fileExists, isDir } from "./files"; +import { delay } from "./throttle"; import { substituteVariables } from "./variables"; -import { PositronApi } from "positron"; +import { LanguageRuntimeMetadata, PositronApi } from "positron"; export async function getPythonInterpreterPath(): Promise { const workspaceFolder = workspace.workspaceFolders?.[0]; @@ -63,7 +64,27 @@ export async function getRInterpreterPath(): Promise { const api = getPositronApi(); if (api) { - const runtime = await api.runtime.getPreferredRuntime("r"); + let runtime: LanguageRuntimeMetadata | undefined; + + // Small number of retries, because getPreferredRuntime + // has its own internal retry logic. + const retries = 3; + const retryInterval = 1000; + + for (let i = 0; i < retries + 1; i++) { + try { + runtime = await api.runtime.getPreferredRuntime("r"); + break; + } catch (error: any) { + // Delay and retry + console.error( + "getPreferredRuntime returned an error; retrying. ", + error, + ); + await delay(retryInterval); + } + } + if (runtime) { const interpreter = runtime.runtimePath; console.log("Using selected R interpreter", interpreter); From d421ed295187ae9d8668bd5f9ab5266adf2457ac Mon Sep 17 00:00:00 2001 From: Michael Marchetti Date: Fri, 23 Aug 2024 13:24:27 -0400 Subject: [PATCH 07/11] fix imports for new vscode utils --- extensions/vscode/src/entrypointTracker.ts | 2 +- extensions/vscode/src/multiStepInputs/newDeployment.ts | 2 +- .../vscode/src/multiStepInputs/selectNewOrExistingConfig.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/extensions/vscode/src/entrypointTracker.ts b/extensions/vscode/src/entrypointTracker.ts index 1b480bdd9..37efe6239 100644 --- a/extensions/vscode/src/entrypointTracker.ts +++ b/extensions/vscode/src/entrypointTracker.ts @@ -17,7 +17,7 @@ import { Contexts } from "src/constants"; import { getPythonInterpreterPath, getRInterpreterPath, -} from "src/utils/config"; +} from "src/utils/vscode"; import { isActiveDocument, relativeDir } from "src/utils/files"; import { hasKnownContentType } from "src/utils/inspect"; import { getSummaryStringFromError } from "src/utils/errors"; diff --git a/extensions/vscode/src/multiStepInputs/newDeployment.ts b/extensions/vscode/src/multiStepInputs/newDeployment.ts index b1bc90ff5..a08eded66 100644 --- a/extensions/vscode/src/multiStepInputs/newDeployment.ts +++ b/extensions/vscode/src/multiStepInputs/newDeployment.ts @@ -31,7 +31,7 @@ import { import { getPythonInterpreterPath, getRInterpreterPath, -} from "src/utils/config"; +} from "src/utils/vscode"; import { getMessageFromError, getSummaryStringFromError, diff --git a/extensions/vscode/src/multiStepInputs/selectNewOrExistingConfig.ts b/extensions/vscode/src/multiStepInputs/selectNewOrExistingConfig.ts index 49373eefa..6a93b75de 100644 --- a/extensions/vscode/src/multiStepInputs/selectNewOrExistingConfig.ts +++ b/extensions/vscode/src/multiStepInputs/selectNewOrExistingConfig.ts @@ -26,7 +26,7 @@ import { import { getPythonInterpreterPath, getRInterpreterPath, -} from "src/utils/config"; +} from "src/utils/vscode"; import { getSummaryStringFromError } from "src/utils/errors"; import { MultiStepInput, From 27c1988ad60b108ac6dd8f5bb59a6ef21e69affb Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Tue, 19 Nov 2024 13:44:16 -0800 Subject: [PATCH 08/11] resolve lint errors --- extensions/vscode/src/utils/vscode.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/extensions/vscode/src/utils/vscode.ts b/extensions/vscode/src/utils/vscode.ts index 2e620c033..104923147 100644 --- a/extensions/vscode/src/utils/vscode.ts +++ b/extensions/vscode/src/utils/vscode.ts @@ -17,7 +17,12 @@ export async function getPythonInterpreterPath(): Promise { "python.interpreterPath", { workspaceFolder: workspaceFolder }, ); - } catch {} + } catch (error: unknown) { + console.error( + "getPythonInterpreterPath was unable to execute command. Error = ", + error, + ); + } if (configuredPython === undefined) { return undefined; } @@ -32,7 +37,7 @@ export async function getPythonInterpreterPath(): Promise { "Scripts/python.exe", "Scripts/python3.exe", ]; - for (let name of names) { + for (const name of names) { const candidate = Uri.joinPath(pythonUri, name); if (await fileExists(candidate)) { python = candidate.fsPath; @@ -75,7 +80,7 @@ export async function getRInterpreterPath(): Promise { try { runtime = await api.runtime.getPreferredRuntime("r"); break; - } catch (error: any) { + } catch (error: unknown) { // Delay and retry console.error( "getPreferredRuntime returned an error; retrying. ", From 1288ef4b74409486e861a0a5f7fc516ce198b6fc Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Tue, 19 Nov 2024 13:45:43 -0800 Subject: [PATCH 09/11] Use the passed in R version for inspection. Also added debug message. --- internal/services/api/post_inspect.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/services/api/post_inspect.go b/internal/services/api/post_inspect.go index 3e79645e9..9fa6f0690 100644 --- a/internal/services/api/post_inspect.go +++ b/internal/services/api/post_inspect.go @@ -80,6 +80,7 @@ func PostInspectHandlerFunc(base util.AbsolutePath, log logging.Logger) http.Han response := []postInspectResponseBody{} log.Debug("Python path to be used for inspection", "path", pythonPath) + log.Debug("R path to be used for inspection", "path", rPath) if req.URL.Query().Get("recursive") == "true" { log.Debug("Recursive inspection intent found") @@ -149,7 +150,7 @@ func PostInspectHandlerFunc(base util.AbsolutePath, log logging.Logger) http.Han // Response already returned by getEntrypointPath return } - configs, err := initialize.GetPossibleConfigs(projectDir, pythonPath, util.Path{}, entrypointPath, log) + configs, err := initialize.GetPossibleConfigs(projectDir, pythonPath, rPath, entrypointPath, log) if err != nil { if aerr, ok := types.IsAgentErrorOf(err, types.ErrorPythonExecNotFound); ok { apiErr := types.APIErrorPythonExecNotFoundFromAgentError(*aerr) From 4d72c19bdcb6a1a02b5fc6509d35423e677afc2a Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Wed, 27 Nov 2024 09:56:54 -0800 Subject: [PATCH 10/11] refactor requiresR to follow requiresPython pattern more closely. --- internal/initialize/initialize.go | 11 +++-------- internal/inspect/r.go | 32 +++++++++++++++++++++---------- internal/types/error.go | 1 + 3 files changed, 26 insertions(+), 18 deletions(-) diff --git a/internal/initialize/initialize.go b/internal/initialize/initialize.go index fcb61371a..636010d68 100644 --- a/internal/initialize/initialize.go +++ b/internal/initialize/initialize.go @@ -59,7 +59,7 @@ func inspectProject(base util.AbsolutePath, python util.Path, rExecutable util.P } cfg.Python = pyConfig } - needR, err := requiresR(cfg, base, rExecutable) + needR, err := requiresR(cfg, base) if err != nil { return nil, err } @@ -92,12 +92,7 @@ func requiresPython(cfg *config.Config, base util.AbsolutePath) (bool, error) { return exists, nil } -func requiresR(cfg *config.Config, base util.AbsolutePath, rExecutable util.Path) (bool, error) { - if rExecutable.String() != "" { - // If user provided R on the command line, - // then configure R for the project. - return true, nil - } +func requiresR(cfg *config.Config, base util.AbsolutePath) (bool, error) { if cfg.R != nil { // InferType returned an R configuration for us to fill in. return true, nil @@ -166,7 +161,7 @@ func normalizeConfig( cfg.Python = pyConfig cfg.Files = append(cfg.Files, fmt.Sprint("/", cfg.Python.PackageFile)) } - needR, err := requiresR(cfg, base, rExecutable) + needR, err := requiresR(cfg, base) if err != nil { log.Debug("Error while determining R as a requirement", "error", err.Error()) return err diff --git a/internal/inspect/r.go b/internal/inspect/r.go index e262ade26..9b11e8eb7 100644 --- a/internal/inspect/r.go +++ b/internal/inspect/r.go @@ -5,8 +5,10 @@ package inspect import ( "bytes" "encoding/json" + "errors" "fmt" "io/fs" + "os" "os/exec" "regexp" "strings" @@ -14,6 +16,7 @@ import ( "github.com/posit-dev/publisher/internal/config" "github.com/posit-dev/publisher/internal/executor" "github.com/posit-dev/publisher/internal/logging" + "github.com/posit-dev/publisher/internal/types" "github.com/posit-dev/publisher/internal/util" ) @@ -163,21 +166,30 @@ func (i *defaultRInspector) getRExecutable() (string, error) { if exists { return i.rExecutable.String(), nil } - return "", fmt.Errorf( + noExecErr := fmt.Errorf( "cannot find the specified R executable %s: %w", i.rExecutable, fs.ErrNotExist) - } else { - // Use whatever is on PATH - path, err := i.pathLooker.LookPath("R") + return "", types.NewAgentError(types.ErrorRExecNotFound, noExecErr, nil) + } + // Find the executable on PATH + var path string + var err error + + i.log.Info("Looking for R on PATH", "PATH", os.Getenv("PATH")) + path, err = i.pathLooker.LookPath("R") + if err == nil { + // Ensure the R is actually runnable. + err = i.validateRExecutable(path) if err == nil { - // Ensure the R is actually runnable. - err = i.validateRExecutable(path) - } - if err != nil { - return "", err + return path, nil } - return path, nil } + + if errors.Is(err, exec.ErrNotFound) { + return "", types.NewAgentError(types.ErrorRExecNotFound, err, nil) + } + + return "", err } var rVersionRE = regexp.MustCompile(`^R version (\d+\.\d+\.\d+)`) diff --git a/internal/types/error.go b/internal/types/error.go index 0154ad6e9..0af9943e4 100644 --- a/internal/types/error.go +++ b/internal/types/error.go @@ -29,6 +29,7 @@ const ( ErrorTomlValidationError ErrorCode = "tomlValidationError" ErrorTomlUnknownError ErrorCode = "tomlUnknownError" ErrorPythonExecNotFound ErrorCode = "pythonExecNotFound" + ErrorRExecNotFound ErrorCode = "rExecNotFound" ) type EventableError interface { From 53829770b3a6e7d2d1ca1b2a5c238c6b530bdec9 Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Mon, 2 Dec 2024 09:22:08 -0800 Subject: [PATCH 11/11] changed reported error --- internal/inspect/r_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/internal/inspect/r_test.go b/internal/inspect/r_test.go index f35aa5a00..0032b8c97 100644 --- a/internal/inspect/r_test.go +++ b/internal/inspect/r_test.go @@ -4,13 +4,13 @@ package inspect import ( "errors" - "io/fs" "os/exec" "runtime" "testing" "github.com/posit-dev/publisher/internal/executor/executortest" "github.com/posit-dev/publisher/internal/logging" + "github.com/posit-dev/publisher/internal/types" "github.com/posit-dev/publisher/internal/util" "github.com/posit-dev/publisher/internal/util/utiltest" "github.com/spf13/afero" @@ -319,9 +319,12 @@ func (s *RSuite) TestGetRExecutableSpecifiedRNotFound() { i := NewRInspector(s.cwd, util.NewPath("/some/R", nil), log) inspector := i.(*defaultRInspector) - executable, err := inspector.getRExecutable() - s.ErrorIs(err, fs.ErrNotExist) + + aerr, yes := types.IsAgentError(err) + s.Equal(yes, true) + s.Equal(aerr.Code, types.ErrorRExecNotFound) + s.Contains(aerr.Message, "Cannot find the specified R executable /some/R: file does not exist.") s.Equal("", executable) }