Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Worker settings #157

Merged
merged 4 commits into from
Oct 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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": [
mofojed marked this conversation as resolved.
Show resolved Hide resolved
{
"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