Skip to content

Commit

Permalink
DHE worker settings (#79-2)
Browse files Browse the repository at this point in the history
  • Loading branch information
bmingles committed Oct 9, 2024
1 parent 28f4059 commit 3ad0113
Show file tree
Hide file tree
Showing 6 changed files with 112 additions and 32 deletions.
43 changes: 42 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,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
23 changes: 17 additions & 6 deletions src/dh/dhe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type {
IdeURL,
QuerySerial,
UniqueID,
WorkerConfig,
WorkerInfo,
WorkerURL,
} from '../types';
Expand Down Expand Up @@ -116,6 +117,7 @@ export function findQueryConfigForSerial(
* 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 @@ -124,6 +126,7 @@ export function findQueryConfigForSerial(
export async function createInteractiveConsoleQuery(
tagId: UniqueID,
dheClient: EnterpriseClient,
workerConfig: WorkerConfig = {},
consoleType?: ConsoleType
): Promise<QuerySerial> {
const userInfo = await dheClient.getUserInfo();
Expand All @@ -137,13 +140,22 @@ 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;
// TODO: deephaven/vscode-deephaven/issues/153 to update this to secure websocket
// connection and remove the http.websockets property
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 timeZone =
Expand All @@ -164,8 +176,7 @@ export async function createInteractiveConsoleQuery(
dbServerName,
heapSize: heapSize,
scheduling,
// TODO: deephaven/vscode-deephaven/issues/153 to update this to secure websocket connection
jvmArgs: '-Dhttp.websockets=true',
jvmArgs,
jvmProfile,
scriptLanguage,
workerKind,
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 3ad0113

Please sign in to comment.