Skip to content

Commit

Permalink
feat: Worker settings (#157)
Browse files Browse the repository at this point in the history
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
    }
  },
]
```
  • Loading branch information
bmingles authored Oct 22, 2024
1 parent f9f1c6c commit 98dbf43
Show file tree
Hide file tree
Showing 6 changed files with 137 additions and 52 deletions.
83 changes: 63 additions & 20 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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/"
]
Expand All @@ -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": []
}
Expand Down
1 change: 1 addition & 0 deletions src/controllers/ExtensionController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ export class ExtensionController implements Disposable {
);

this._dheServiceFactory = DheService.factory(
this._config,
this._coreCredentialsCache,
this._dheClientCache,
this._dheCredentialsCache,
Expand Down
28 changes: 21 additions & 7 deletions src/dh/dhe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type {
IdeURL,
QuerySerial,
UniqueID,
WorkerConfig,
WorkerInfo,
WorkerURL,
} from '../types';
Expand Down Expand Up @@ -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
Expand All @@ -114,6 +116,7 @@ export async function hasInteractivePermission(
export async function createInteractiveConsoleQuery(
tagId: UniqueID,
dheClient: EnterpriseClient,
workerConfig: WorkerConfig = {},
consoleType?: ConsoleType
): Promise<QuerySerial> {
const userInfo = await dheClient.getUserInfo();
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
44 changes: 21 additions & 23 deletions src/services/ConfigService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) }));
}

Expand All @@ -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,
Expand Down
19 changes: 19 additions & 0 deletions src/services/DheService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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<Lazy<DhcType.LoginCredentials>>,
dheClientCache: IAsyncCacheService<URL, EnterpriseClient>,
dheCredentialsCache: URLMap<DheLoginCredentials>,
Expand All @@ -51,6 +54,7 @@ export class DheService implements IDheService {
create: (serverUrl: URL): IDheService =>
new DheService(
serverUrl,
configService,
coreCredentialsCache,
dheClientCache,
dheCredentialsCache,
Expand All @@ -65,12 +69,14 @@ export class DheService implements IDheService {
*/
private constructor(
serverUrl: URL,
configService: IConfigService,
coreCredentialsCache: URLMap<Lazy<DhcType.LoginCredentials>>,
dheClientCache: IAsyncCacheService<URL, EnterpriseClient>,
dheCredentialsCache: URLMap<DheLoginCredentials>,
dheJsApiCache: IAsyncCacheService<URL, DheType>
) {
this.serverUrl = serverUrl;
this._config = configService;
this._coreCredentialsCache = coreCredentialsCache;
this._dheClientCache = dheClientCache;
this._dheCredentialsCache = dheCredentialsCache;
Expand All @@ -81,6 +87,7 @@ export class DheService implements IDheService {

private _clientPromise: Promise<EnterpriseClient | null> | null = null;
private _isConnected: boolean = false;
private readonly _config: IConfigService;
private readonly _coreCredentialsCache: URLMap<
Lazy<DhcType.LoginCredentials>
>;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -210,6 +228,7 @@ export class DheService implements IDheService {
const querySerial = await createInteractiveConsoleQuery(
tagId,
dheClient,
this.getWorkerConfig(),
consoleType
);
this._querySerialSet.add(querySerial);
Expand Down
14 changes: 12 additions & 2 deletions src/types/commonTypes.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,28 @@ 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 =
| CoreConnectionConfig
| EnterpriseConnectionConfig
| URL;

export interface WorkerConfig {
dbServerName?: string;
heapSize?: number;
jvmArgs?: string;
jvmProfile?: string;
scriptLanguage?: string;
}

export interface ConnectionState {
readonly isConnected: boolean;
readonly serverUrl: URL;
Expand Down

0 comments on commit 98dbf43

Please sign in to comment.