From 98dbf439e82f4125963ee581f61388bd4794f874 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Tue, 22 Oct 2024 15:25:26 -0500 Subject: [PATCH] feat: Worker settings (#157) Enterprise settings now support an expanded config: - Optional server label - Experimental worker config - an object of optional worker settings that will drive PQ creation Also fixed a small json schema bug for community settings that was breaking intellisense in settings.json **Testing** Simple config should still work: ```jsonc "deephaven.enterpriseServers": [ https://dev-vplus.int.illumon.com:8123/ ] ``` Expanded config should drive server label in the server tree and / or worker creation settings: ```jsonc "deephaven.enterpriseServers": [ { "url": "https://dev-vplus.int.illumon.com:8123/", "label": "San Luis", // optional label "experimentalWorkerConfig": { // custom heap size "heapSize": 0.5 } }, ] ``` --- package.json | 83 +++++++++++++++++++------- src/controllers/ExtensionController.ts | 1 + src/dh/dhe.ts | 28 ++++++--- src/services/ConfigService.ts | 44 +++++++------- src/services/DheService.ts | 19 ++++++ src/types/commonTypes.d.ts | 14 ++++- 6 files changed, 137 insertions(+), 52 deletions(-) diff --git a/package.json b/package.json index 052ab35f..dc056c28 100644 --- a/package.json +++ b/package.json @@ -57,27 +57,29 @@ "deephaven.coreServers": { "description": "List of Deephaven Core servers that the extension can connect to.", "type": "array", - "items": [ - { - "type": "string", - "description": "Deephaven Core server URL" - }, - { - "type": "object", - "description": "Deephaven Core server config", - "properties": { - "url": { - "type": "string", - "description": "Deephaven Core server URL" - }, - "label": { - "type": "string", - "title": "Label", - "description": "Optional label for the server" + "items": { + "anyOf": [ + { + "type": "string", + "description": "Deephaven Core server URL" + }, + { + "type": "object", + "description": "Deephaven Core server config", + "properties": { + "url": { + "type": "string", + "description": "Deephaven Core server URL" + }, + "label": { + "type": "string", + "title": "Label", + "description": "Optional label for the server" + } } } - } - ], + ] + }, "default": [ "http://localhost:10000/" ] @@ -86,7 +88,48 @@ "description": "List of Deephaven Enterprise servers that the extension can connect to.", "type": "array", "items": { - "type": "string" + "anyOf": [ + { + "type": "string", + "description": "Deephaven Enterprise server URL" + }, + { + "type": "object", + "description": "Deephaven Enterprise server config", + "properties": { + "url": { + "type": "string", + "description": "Deephaven Enterprise server URL" + }, + "label": { + "type": "string", + "title": "Label", + "description": "Optional label for the server" + }, + "experimentalWorkerConfig": { + "type": "object", + "description": "(experimental) Worker configuration used when creating new connections to the server", + "properties": { + "dbServerName": { + "type": "string" + }, + "heapSize": { + "type": "number" + }, + "jvmArgs": { + "type": "string" + }, + "jvmProfile": { + "type": "string" + }, + "scriptLanguage": { + "type": "string" + } + } + } + } + } + ] }, "default": [] } diff --git a/src/controllers/ExtensionController.ts b/src/controllers/ExtensionController.ts index 6b7823f6..53218df7 100644 --- a/src/controllers/ExtensionController.ts +++ b/src/controllers/ExtensionController.ts @@ -311,6 +311,7 @@ export class ExtensionController implements Disposable { ); this._dheServiceFactory = DheService.factory( + this._config, this._coreCredentialsCache, this._dheClientCache, this._dheCredentialsCache, diff --git a/src/dh/dhe.ts b/src/dh/dhe.ts index ffc906d2..804bb38a 100644 --- a/src/dh/dhe.ts +++ b/src/dh/dhe.ts @@ -12,6 +12,7 @@ import type { IdeURL, QuerySerial, UniqueID, + WorkerConfig, WorkerInfo, WorkerURL, } from '../types'; @@ -106,6 +107,7 @@ export async function hasInteractivePermission( * Create a query of type `InteractiveConsole`. * @param tagId Unique tag id to include in the query name. * @param dheClient The DHE client to use to create the query. + * @param workerConfig Worker configuration overrides. * @param consoleType The type of console to create. * @returns A promise that resolves to the serial of the created query. Note * that this will resolve before the query is actually ready to use. Use @@ -114,6 +116,7 @@ export async function hasInteractivePermission( export async function createInteractiveConsoleQuery( tagId: UniqueID, dheClient: EnterpriseClient, + workerConfig: WorkerConfig = {}, consoleType?: ConsoleType ): Promise { const userInfo = await dheClient.getUserInfo(); @@ -130,13 +133,26 @@ export async function createInteractiveConsoleQuery( ]); const name = `IC - VS Code - ${tagId}`; - const dbServerName = dbServers[0]?.name ?? 'Query 1'; - const heapSize = queryConstants.pqDefaultHeap; - const jvmProfile = serverConfigValues.jvmProfileDefault; + const dbServerName = + workerConfig?.dbServerName ?? dbServers[0]?.name ?? 'Query 1'; + const heapSize = workerConfig?.heapSize ?? queryConstants.pqDefaultHeap; + + // We have to use websockets since fetch over http2 is not sufficiently + // supported in the nodejs environment bundled with `vscode` (v20 at time of + // this comment). Note that the `http` in the key name does not indicate + // insecure websockets. The connection will be a `wss:` secure connection. + const jvmArgs = workerConfig?.jvmArgs + ? `'-Dhttp.websockets=true' ${workerConfig.jvmArgs}` + : '-Dhttp.websockets=true'; + + const jvmProfile = + workerConfig?.jvmProfile ?? serverConfigValues.jvmProfileDefault; const scriptLanguage = + workerConfig?.scriptLanguage ?? serverConfigValues.scriptSessionProviders?.find( p => p.toLocaleLowerCase() === consoleType - ) ?? 'Python'; + ) ?? + 'Python'; const workerKind = serverConfigValues.workerKinds?.[0]?.name; const autoDelete = autoDeleteTimeoutMs > 0; @@ -166,9 +182,7 @@ export async function createInteractiveConsoleQuery( dbServerName, heapSize: heapSize, scheduling, - // We have to use websockets since http2 is not sufficiently supported in - // the nodejs environment (v20 at time of this comment). - jvmArgs: '-Dhttp.websockets=true', + jvmArgs, jvmProfile, scriptLanguage, timeout, diff --git a/src/services/ConfigService.ts b/src/services/ConfigService.ts index d90d16ad..bc0f9124 100644 --- a/src/services/ConfigService.ts +++ b/src/services/ConfigService.ts @@ -29,17 +29,7 @@ function getCoreServers(): CoreConnectionConfig[] { logger.info('Core servers:', JSON.stringify(expandedConfig)); return expandedConfig - .filter(server => { - try { - // Filter out any invalid server configs to avoid crashing the extension - // further upstream. - new URL(server.url); - return true; - } catch (err) { - logger.error(err, server.url); - return false; - } - }) + .filter(hasValidURL) .map(server => ({ ...server, url: new URL(server.url) })); } @@ -49,25 +39,33 @@ function getEnterpriseServers(): EnterpriseConnectionConfig[] { [] ); - const expandedConfig = config.map(url => ({ url })); + // Expand each server config to a full `ConnectionConfig` object. + const expandedConfig = config.map(server => + typeof server === 'string' ? { url: server } : server + ); logger.info('Enterprise servers:', JSON.stringify(expandedConfig)); return expandedConfig - .filter(server => { - try { - // Filter out any invalid server configs to avoid crashing the extension - // further upstream. - new URL(server.url); - return true; - } catch (err) { - logger.error(err, server.url); - return false; - } - }) + .filter(hasValidURL) .map(server => ({ ...server, url: new URL(server.url) })); } +/** + * Attempt to parse a `url` string prop into a `URL` object. + * @param objectWithUrl An object with a `url` string prop. + * @returns `true` if the `url` prop is a valid URL, `false` otherwise. + */ +function hasValidURL({ url }: { url: string }): boolean { + try { + new URL(url); + return true; + } catch (err) { + logger.error(err, url); + return false; + } +} + // eslint-disable-next-line @typescript-eslint/naming-convention export const ConfigService: IConfigService = { getCoreServers, diff --git a/src/services/DheService.ts b/src/services/DheService.ts index d7dc26f0..b8790297 100644 --- a/src/services/DheService.ts +++ b/src/services/DheService.ts @@ -9,11 +9,13 @@ import { WorkerURL, type ConsoleType, type IAsyncCacheService, + type IConfigService, type IDheService, type IDheServiceFactory, type Lazy, type QuerySerial, type UniqueID, + type WorkerConfig, type WorkerInfo, } from '../types'; import { URLMap } from './URLMap'; @@ -42,6 +44,7 @@ export class DheService implements IDheService { * @returns A factory function that can be used to create DheService instances. */ static factory = ( + configService: IConfigService, coreCredentialsCache: URLMap>, dheClientCache: IAsyncCacheService, dheCredentialsCache: URLMap, @@ -51,6 +54,7 @@ export class DheService implements IDheService { create: (serverUrl: URL): IDheService => new DheService( serverUrl, + configService, coreCredentialsCache, dheClientCache, dheCredentialsCache, @@ -65,12 +69,14 @@ export class DheService implements IDheService { */ private constructor( serverUrl: URL, + configService: IConfigService, coreCredentialsCache: URLMap>, dheClientCache: IAsyncCacheService, dheCredentialsCache: URLMap, dheJsApiCache: IAsyncCacheService ) { this.serverUrl = serverUrl; + this._config = configService; this._coreCredentialsCache = coreCredentialsCache; this._dheClientCache = dheClientCache; this._dheCredentialsCache = dheCredentialsCache; @@ -81,6 +87,7 @@ export class DheService implements IDheService { private _clientPromise: Promise | null = null; private _isConnected: boolean = false; + private readonly _config: IConfigService; private readonly _coreCredentialsCache: URLMap< Lazy >; @@ -152,6 +159,17 @@ export class DheService implements IDheService { } }; + /** + * Get the config for creating new workers. + * @returns Worker config or undefined if not found. + */ + getWorkerConfig = (): WorkerConfig | undefined => { + return this._config + .getEnterpriseServers() + .find(server => server.url.toString() === this.serverUrl.toString()) + ?.experimentalWorkerConfig; + }; + /** * Get worker info for given worker URL. * @param workerUrl Worker URL to get info for. @@ -210,6 +228,7 @@ export class DheService implements IDheService { const querySerial = await createInteractiveConsoleQuery( tagId, dheClient, + this.getWorkerConfig(), consoleType ); this._querySerialSet.add(querySerial); diff --git a/src/types/commonTypes.d.ts b/src/types/commonTypes.d.ts index 8a8940c7..77347026 100644 --- a/src/types/commonTypes.d.ts +++ b/src/types/commonTypes.d.ts @@ -28,11 +28,13 @@ export interface CoreConnectionConfig { } export type EnterpriseConnectionConfigStored = - Brand<'EnterpriseConnectionConfigStored'>; + | Brand<'EnterpriseConnectionConfigStored'> + | { url: string; label?: string; experimentalWorkerConfig?: WorkerConfig }; export interface EnterpriseConnectionConfig { - label?: string; url: URL; + label?: string; + experimentalWorkerConfig?: WorkerConfig; } export type ServerConnectionConfig = @@ -40,6 +42,14 @@ export type ServerConnectionConfig = | EnterpriseConnectionConfig | URL; +export interface WorkerConfig { + dbServerName?: string; + heapSize?: number; + jvmArgs?: string; + jvmProfile?: string; + scriptLanguage?: string; +} + export interface ConnectionState { readonly isConnected: boolean; readonly serverUrl: URL;