From 1a75b52b7450732dfa9ada0f5f28e4b6a177ae61 Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Mon, 8 Apr 2024 16:12:30 +0200 Subject: [PATCH 1/8] refactor: move availability check into runner --- .../safe-ds-lang/src/language/runner/safe-ds-runner.ts | 5 +++++ packages/safe-ds-vscode/src/extension/mainClient.ts | 8 +------- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/safe-ds-lang/src/language/runner/safe-ds-runner.ts b/packages/safe-ds-lang/src/language/runner/safe-ds-runner.ts index 299604342..717b476a7 100644 --- a/packages/safe-ds-lang/src/language/runner/safe-ds-runner.ts +++ b/packages/safe-ds-lang/src/language/runner/safe-ds-runner.ts @@ -136,6 +136,11 @@ export class SafeDsRunner { * Uses the 'safe-ds.runner.command' setting to execute the process. */ public async startPythonServer(): Promise { + if (this.isPythonServerAvailable()) { + this.info('As the Safe-DS Runner is currently successfully running, no attempt to start it will be made'); + return; + } + this.acceptsConnections = false; const runnerCommandParts = this.runnerCommand.split(/\s/u); const runnerCommand = runnerCommandParts.shift()!; // After shift, only the actual args are left diff --git a/packages/safe-ds-vscode/src/extension/mainClient.ts b/packages/safe-ds-vscode/src/extension/mainClient.ts index 559eecf47..f64bc16b3 100644 --- a/packages/safe-ds-vscode/src/extension/mainClient.ts +++ b/packages/safe-ds-vscode/src/extension/mainClient.ts @@ -428,13 +428,7 @@ const registerVSCodeWatchers = function () { services.runtime.Runner.updateRunnerCommand( vscode.workspace.getConfiguration('safe-ds.runner').get('command')!, ); - if (!services.runtime.Runner.isPythonServerAvailable()) { - services.runtime.Runner.startPythonServer(); - } else { - logOutput( - 'As the Safe-DS Runner is currently successfully running, no attempt to start it will be made', - ); - } + services.runtime.Runner.startPythonServer(); } }); }; From 6de24ee7cf23f458006b51afbe2bc2cf0a71d5c0 Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Mon, 8 Apr 2024 16:48:23 +0200 Subject: [PATCH 2/8] refactor: only start the runner process in the language server --- .../src/language/runner/safe-ds-runner.ts | 16 ++++++++++++++++ .../safe-ds-vscode/src/extension/mainClient.ts | 7 ++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/packages/safe-ds-lang/src/language/runner/safe-ds-runner.ts b/packages/safe-ds-lang/src/language/runner/safe-ds-runner.ts index 717b476a7..61bfb30a0 100644 --- a/packages/safe-ds-lang/src/language/runner/safe-ds-runner.ts +++ b/packages/safe-ds-lang/src/language/runner/safe-ds-runner.ts @@ -64,6 +64,9 @@ export class SafeDsRunner { services.workspace.SettingsProvider.onRunnerCommandUpdate(async (newValue) => { this.updateRunnerCommand(newValue); await this.startPythonServer(); + if (this.isPythonServerAvailable()) { + await this.messaging.sendNotification('runner/started', this.port); + } }); services.shared.lsp.Connection?.onShutdown(async () => { @@ -119,6 +122,19 @@ export class SafeDsRunner { } /* c8 ignore stop */ + async connectToPort(port: number): Promise { + if (this.isPythonServerAvailable()) { + return; + } + + this.port = port; + try { + await this.connectToWebSocket(); + } catch (error) { + await this.stopPythonServer(); + } + } + /** * Change the command to start the runner process. This will not cause the runner process to restart, if it is already running. * diff --git a/packages/safe-ds-vscode/src/extension/mainClient.ts b/packages/safe-ds-vscode/src/extension/mainClient.ts index f64bc16b3..db518f1e4 100644 --- a/packages/safe-ds-vscode/src/extension/mainClient.ts +++ b/packages/safe-ds-vscode/src/extension/mainClient.ts @@ -37,7 +37,12 @@ export const activate = async function (context: vscode.ExtensionContext) { }, }) ).SafeDs; - await services.runtime.Runner.startPythonServer(); + + client.onNotification('runner/started', async (port: number) => { + await services.runtime.Runner.connectToPort(port); + }); + + // await services.runtime.Runner.startPythonServer(); acceptRunRequests(context); }; From 6dcd609785a2d6f7de70fb5549e2822a5dda06af Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Mon, 8 Apr 2024 16:57:17 +0200 Subject: [PATCH 3/8] fix: ensure we cannot miss the `runner/started` notification --- .../safe-ds-vscode/src/extension/mainClient.ts | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/packages/safe-ds-vscode/src/extension/mainClient.ts b/packages/safe-ds-vscode/src/extension/mainClient.ts index db518f1e4..2693385b3 100644 --- a/packages/safe-ds-vscode/src/extension/mainClient.ts +++ b/packages/safe-ds-vscode/src/extension/mainClient.ts @@ -23,27 +23,25 @@ let lastSuccessfulPipelineNode: ast.SdsPipeline | undefined; // This function is called when the extension is activated. export const activate = async function (context: vscode.ExtensionContext) { initializeLog(); - client = startLanguageClient(context); - const runnerCommandSetting = vscode.workspace.getConfiguration('safe-ds.runner').get('command')!; // Default is set services = ( await createSafeDsServices(NodeFileSystem, { logger: { info: logOutput, error: logError, }, - runnerCommand: runnerCommandSetting, userMessageProvider: { showErrorMessage: vscode.window.showErrorMessage, }, }) ).SafeDs; + client = createLanguageClient(context); client.onNotification('runner/started', async (port: number) => { await services.runtime.Runner.connectToPort(port); }); + await client.start(); - // await services.runtime.Runner.startPythonServer(); - acceptRunRequests(context); + await acceptRunRequests(context); }; // This function is called when the extension is deactivated. @@ -55,7 +53,7 @@ export const deactivate = async function (): Promise { return; }; -const startLanguageClient = function (context: vscode.ExtensionContext): LanguageClient { +const createLanguageClient = function (context: vscode.ExtensionContext): LanguageClient { const serverModule = context.asAbsolutePath(path.join('dist', 'extension', 'mainServer.cjs')); // The debug options for the server // --inspect=6009: runs the server in Node's Inspector mode so VS Code can attach to the server for debugging. @@ -87,12 +85,8 @@ const startLanguageClient = function (context: vscode.ExtensionContext): Languag outputChannel: vscode.window.createOutputChannel('Safe-DS Language Client', 'log'), }; - // Create the language client and start the client. - const result = new LanguageClient('safe-ds', 'Safe-DS', serverOptions, clientOptions); - - // Start the client. This will also launch the server - result.start(); - return result; + // Create the language client + return new LanguageClient('safe-ds', 'Safe-DS', serverOptions, clientOptions); }; const acceptRunRequests = async function (context: vscode.ExtensionContext) { From b207daf3ea0d48ce141c7d1dc7c367a10b31acfd Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Mon, 8 Apr 2024 16:58:08 +0200 Subject: [PATCH 4/8] refactor: remove configuration watcher in client --- .../safe-ds-vscode/src/extension/mainClient.ts | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/packages/safe-ds-vscode/src/extension/mainClient.ts b/packages/safe-ds-vscode/src/extension/mainClient.ts index 2693385b3..721d8ca1f 100644 --- a/packages/safe-ds-vscode/src/extension/mainClient.ts +++ b/packages/safe-ds-vscode/src/extension/mainClient.ts @@ -92,8 +92,6 @@ const createLanguageClient = function (context: vscode.ExtensionContext): Langua const acceptRunRequests = async function (context: vscode.ExtensionContext) { // Register VS Code Entry Points registerVSCodeCommands(context); - // Register watchers - registerVSCodeWatchers(); }; const registerVSCodeCommands = function (context: vscode.ExtensionContext) { @@ -419,19 +417,6 @@ const validateDocuments = async function ( return undefined; }; -const registerVSCodeWatchers = function () { - vscode.workspace.onDidChangeConfiguration((event) => { - if (event.affectsConfiguration('safe-ds.runner.command')) { - // Try starting runner - logOutput('Safe-DS Runner Command was updated'); - services.runtime.Runner.updateRunnerCommand( - vscode.workspace.getConfiguration('safe-ds.runner').get('command')!, - ); - services.runtime.Runner.startPythonServer(); - } - }); -}; - const isRangeEqual = function (lhs: Range, rhs: Range): boolean { return ( lhs.start.character === rhs.start.character && From 2d73e245d3bdd4d1b2bf7b6b893ecf283df55c8c Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Mon, 8 Apr 2024 17:17:19 +0200 Subject: [PATCH 5/8] refactor: export RPC method names --- packages/safe-ds-lang/src/language/index.ts | 7 +++++++ .../safe-ds-lang/src/language/runner/safe-ds-runner.ts | 4 +++- packages/safe-ds-vscode/src/extension/mainClient.ts | 4 ++-- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/safe-ds-lang/src/language/index.ts b/packages/safe-ds-lang/src/language/index.ts index c4ad95b27..1bdf16c6c 100644 --- a/packages/safe-ds-lang/src/language/index.ts +++ b/packages/safe-ds-lang/src/language/index.ts @@ -1,3 +1,5 @@ +import { RPC_RUNNER_STARTED } from './runner/safe-ds-runner.js'; + // Services export type { SafeDsServices } from './safe-ds-module.js'; export { createSafeDsServices } from './safe-ds-module.js'; @@ -17,3 +19,8 @@ export { locationToString, positionToString, rangeToString } from '../helpers/lo // Messages export * as messages from './runner/messages.js'; + +// Remote procedure calls +export const rpc = { + runnerStarted: RPC_RUNNER_STARTED, +}; diff --git a/packages/safe-ds-lang/src/language/runner/safe-ds-runner.ts b/packages/safe-ds-lang/src/language/runner/safe-ds-runner.ts index 61bfb30a0..5aa75540f 100644 --- a/packages/safe-ds-lang/src/language/runner/safe-ds-runner.ts +++ b/packages/safe-ds-lang/src/language/runner/safe-ds-runner.ts @@ -22,6 +22,8 @@ import { SafeDsMessagingProvider } from '../lsp/safe-ds-messaging-provider.js'; // Most of the functionality cannot be tested automatically as a functioning runner setup would always be required +export const RPC_RUNNER_STARTED = 'runner/started'; + const LOWEST_SUPPORTED_VERSION = '0.8.0'; const LOWEST_UNSUPPORTED_VERSION = '0.9.0'; const npmVersionRange = `>=${LOWEST_SUPPORTED_VERSION} <${LOWEST_UNSUPPORTED_VERSION}`; @@ -65,7 +67,7 @@ export class SafeDsRunner { this.updateRunnerCommand(newValue); await this.startPythonServer(); if (this.isPythonServerAvailable()) { - await this.messaging.sendNotification('runner/started', this.port); + await this.messaging.sendNotification(RPC_RUNNER_STARTED, this.port); } }); diff --git a/packages/safe-ds-vscode/src/extension/mainClient.ts b/packages/safe-ds-vscode/src/extension/mainClient.ts index 721d8ca1f..88d15f5d0 100644 --- a/packages/safe-ds-vscode/src/extension/mainClient.ts +++ b/packages/safe-ds-vscode/src/extension/mainClient.ts @@ -2,7 +2,7 @@ import * as path from 'node:path'; import * as vscode from 'vscode'; import type { LanguageClientOptions, ServerOptions } from 'vscode-languageclient/node.js'; import { LanguageClient, TransportKind } from 'vscode-languageclient/node.js'; -import { ast, createSafeDsServices, getModuleMembers, messages, SafeDsServices } from '@safe-ds/lang'; +import { ast, createSafeDsServices, getModuleMembers, messages, rpc, SafeDsServices } from '@safe-ds/lang'; import { NodeFileSystem } from 'langium/node'; import { initializeLog, logError, logOutput, printOutputMessage } from './output.js'; import crypto from 'crypto'; @@ -36,7 +36,7 @@ export const activate = async function (context: vscode.ExtensionContext) { ).SafeDs; client = createLanguageClient(context); - client.onNotification('runner/started', async (port: number) => { + client.onNotification(rpc.runnerStarted, async (port: number) => { await services.runtime.Runner.connectToPort(port); }); await client.start(); From 7c69ff48b691457cef6de068af082f398fd0af81 Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Mon, 8 Apr 2024 17:23:38 +0200 Subject: [PATCH 6/8] fix: also start runner when runner command is passed when services are created --- .../src/language/runner/safe-ds-runner.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/safe-ds-lang/src/language/runner/safe-ds-runner.ts b/packages/safe-ds-lang/src/language/runner/safe-ds-runner.ts index 5aa75540f..1dfe88724 100644 --- a/packages/safe-ds-lang/src/language/runner/safe-ds-runner.ts +++ b/packages/safe-ds-lang/src/language/runner/safe-ds-runner.ts @@ -64,11 +64,7 @@ export class SafeDsRunner { this.registerMessageLoggingCallbacks(); services.workspace.SettingsProvider.onRunnerCommandUpdate(async (newValue) => { - this.updateRunnerCommand(newValue); - await this.startPythonServer(); - if (this.isPythonServerAvailable()) { - await this.messaging.sendNotification(RPC_RUNNER_STARTED, this.port); - } + await this.updateRunnerCommand(newValue); }); services.shared.lsp.Connection?.onShutdown(async () => { @@ -143,9 +139,13 @@ export class SafeDsRunner { * @param command New Runner Command. */ /* c8 ignore start */ - public updateRunnerCommand(command: string | undefined): void { + public async updateRunnerCommand(command: string | undefined): Promise { if (command) { this.runnerCommand = command; + await this.startPythonServer(); + if (this.isPythonServerAvailable()) { + await this.messaging.sendNotification(RPC_RUNNER_STARTED, this.port); + } } } From fcb74a3e71789b30124698a6a0c9cb04ca012489 Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Mon, 8 Apr 2024 17:27:01 +0200 Subject: [PATCH 7/8] refactor: inline a method --- packages/safe-ds-vscode/src/extension/mainClient.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/safe-ds-vscode/src/extension/mainClient.ts b/packages/safe-ds-vscode/src/extension/mainClient.ts index 88d15f5d0..0cf3202c0 100644 --- a/packages/safe-ds-vscode/src/extension/mainClient.ts +++ b/packages/safe-ds-vscode/src/extension/mainClient.ts @@ -41,7 +41,7 @@ export const activate = async function (context: vscode.ExtensionContext) { }); await client.start(); - await acceptRunRequests(context); + registerVSCodeCommands(context); }; // This function is called when the extension is deactivated. @@ -89,11 +89,6 @@ const createLanguageClient = function (context: vscode.ExtensionContext): Langua return new LanguageClient('safe-ds', 'Safe-DS', serverOptions, clientOptions); }; -const acceptRunRequests = async function (context: vscode.ExtensionContext) { - // Register VS Code Entry Points - registerVSCodeCommands(context); -}; - const registerVSCodeCommands = function (context: vscode.ExtensionContext) { const registerCommandWithCheck = (commandId: string, callback: (...args: any[]) => Promise) => { return vscode.commands.registerCommand(commandId, (...args: any[]) => { From 6b2afb2afe1217a5c9c7095e99f23138cf2b21aa Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Mon, 8 Apr 2024 17:34:28 +0200 Subject: [PATCH 8/8] test: ignore lines for coverage --- packages/safe-ds-lang/src/language/runner/safe-ds-runner.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/safe-ds-lang/src/language/runner/safe-ds-runner.ts b/packages/safe-ds-lang/src/language/runner/safe-ds-runner.ts index 1dfe88724..3e9c362aa 100644 --- a/packages/safe-ds-lang/src/language/runner/safe-ds-runner.ts +++ b/packages/safe-ds-lang/src/language/runner/safe-ds-runner.ts @@ -118,7 +118,6 @@ export class SafeDsRunner { ); }, 'runtime_error'); } - /* c8 ignore stop */ async connectToPort(port: number): Promise { if (this.isPythonServerAvailable()) { @@ -138,7 +137,6 @@ export class SafeDsRunner { * * @param command New Runner Command. */ - /* c8 ignore start */ public async updateRunnerCommand(command: string | undefined): Promise { if (command) { this.runnerCommand = command;