Skip to content

Commit 64c55b0

Browse files
authored
fix: DH-21024: Grizzly worker kind fixes (#266)
DH-21024: Grizzly worker kind fixes This PR includes a couple of fixes + configuration improvements: - Worker kinds are now filtered to only use Core+ workers. This fixes bug where legacy worker sorted first would get picked - Fixed a bug where `operateAs` was redeclared in an if block instead of being set in the outer. This was causing operateAs to be ignored. Added respective unit tests - Added additional `experimentalWorkerConfig` settings: - `additionalMemory` - `classPaths` - `engine` - `envVars` - I did not add support for `enableGcLogs` or `python environment` as they require additional logic I didn't want to have to replicate from the web ### Testing Config Configure a grizzly server such that legacy workers are sorted to beginning of worker kind list. See description in the Jira or you can use https://bmingles-f3.int.illumon.com:8123/iriside/ is already which is already setup for this. #### VS Code `deephaven.enterpriseServers` settings for different tests - Default config ```json { "url": "https://bmingles-f3.int.illumon.com:8123/", "experimentalWorkerConfig": { "heapSize": 0.5 } }, ``` Connecting to server should successfully create a connection - Named config ```json { "url": "https://bmingles-f3.int.illumon.com:8123/", "experimentalWorkerConfig": { "engine": "Core+", "heapSize": 0.5 } }, ``` Connecting to server should successfully create a connection - Additional config ```json { "url": "https://bmingles-f3.int.illumon.com:8123/", "experimentalWorkerConfig": { "additionalMemory": 1, "classPaths": "", "envVars": "FOO=BAR", "heapSize": 0.5 } }, ``` - Connecting to server should successfully create a connection. - additionalMemory can be seen in query monitor column - envVars ```py import os print(os.environ.get('FOO')) ``` - classPaths - not sure how to test this one ### Testing operateAs - Disconnect from grizzly server if connect. - Right-click on the server in VS Code, and `Connect to Server as Another User` - Login as iris, and give another user to operateAs - Should see the `operateAs` user in the `owner` column of query monitor for the interactive console query that gets created
1 parent bb985db commit 64c55b0

File tree

9 files changed

+183
-18
lines changed

9 files changed

+183
-18
lines changed

__mocks__/vscode.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,7 @@ export const window = {
178178
registerFileDecorationProvider: vi
179179
.fn()
180180
.mockName('registerFileDecorationProvider'),
181+
showInputBox: vi.fn().mockName('showInputBox'),
181182
};
182183

183184
export const workspace = {

package.json

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,9 +110,23 @@
110110
"type": "object",
111111
"description": "(experimental) Worker configuration used when creating new connections to the server",
112112
"properties": {
113+
"additionalMemory": {
114+
"type": "number",
115+
"description": "(GB) The amount of additional memory allocated to this worker process beyond the JVM heap memory. This allows Deephaven to account for memory allocated by Python native code, Java direct memory, and other types of non-heap memory."
116+
},
117+
"classPaths": {
118+
"type": "string"
119+
},
113120
"dbServerName": {
114121
"type": "string"
115122
},
123+
"engine": {
124+
"type": "string",
125+
"description": "Core+ engine configuration name"
126+
},
127+
"envVars": {
128+
"type": "string"
129+
},
116130
"heapSize": {
117131
"type": "number"
118132
},
-3.85 MB
Binary file not shown.

src/common/constants.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,3 +192,11 @@ export const DH_PYTHON_REMOTE_SOURCE_PLUGIN_NAME =
192192
export const DH_PROTECTED_VARIABLE_NAMES: Set<string> = new Set([
193193
DH_PYTHON_REMOTE_SOURCE_PLUGIN_VARIABLE,
194194
]);
195+
196+
export const PROTOCOL = {
197+
/* eslint-disable @typescript-eslint/naming-convention */
198+
COMMUNITY: 'Community',
199+
ENTERPRISE_COMM: 'EnterpriseComm',
200+
ENTERPRISE_WEBSOCKET: 'EnterpriseWebsocket',
201+
/* eslint-enable @typescript-eslint/naming-convention */
202+
} as const;

src/controllers/UserLoginController.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,7 @@ export class UserLoginController extends ControllerBase {
326326
title,
327327
userLoginPreferences,
328328
privateKeyUserNames,
329-
showOperatesAs: operateAsAnotherUser,
329+
showOperateAs: operateAsAnotherUser,
330330
});
331331

332332
// Cancelled by user

src/dh/dhe.ts

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import type {
55
EnterpriseClient,
66
QueryInfo,
77
TypeSpecificFields,
8+
WorkerKind,
89
} from '@deephaven-enterprise/jsapi-types';
910
import { DraftQuery, QueryScheduler } from '@deephaven-enterprise/query-utils';
1011
import type {
@@ -36,6 +37,7 @@ import {
3637
DHE_FEATURES_URL_PATH,
3738
INTERACTIVE_CONSOLE_QUERY_TYPE,
3839
INTERACTIVE_CONSOLE_TEMPORARY_QUEUE_NAME,
40+
PROTOCOL,
3941
UnsupportedFeatureQueryError,
4042
} from '../common';
4143
import { withResolvers } from '../util';
@@ -203,7 +205,7 @@ export async function createInteractiveConsoleQuery(
203205
consoleType?: ConsoleType
204206
): Promise<QuerySerial> {
205207
const userInfo = await dheClient.getUserInfo();
206-
const owner = userInfo.username;
208+
const owner = userInfo.operateAs;
207209
const type = INTERACTIVE_CONSOLE_QUERY_TYPE;
208210
const queueName = INTERACTIVE_CONSOLE_TEMPORARY_QUEUE_NAME;
209211
const autoDeleteTimeoutMs = DEFAULT_TEMPORARY_QUERY_AUTO_TIMEOUT_MS;
@@ -215,6 +217,27 @@ export async function createInteractiveConsoleQuery(
215217
dheClient.getServerConfigValues(),
216218
]);
217219

220+
const coreWorkerKinds = serverConfigValues.workerKinds.filter(
221+
isCommunityWorkerKind
222+
);
223+
224+
const engine = (
225+
workerConfig.engine == null
226+
? coreWorkerKinds[0]
227+
: coreWorkerKinds.find(({ name, title }) =>
228+
// Match on title first since it's the most likely to be configured by
229+
// a user, but fallback to name just in case someone is using the actual
230+
// worker kind name.
231+
[title, name].includes(workerConfig.engine!)
232+
)
233+
)?.name;
234+
235+
if (engine == null) {
236+
throw new Error(
237+
`No Core+ engine configurations were found${workerConfig.engine == null ? '' : ` matching '${workerConfig.engine}'`}.`
238+
);
239+
}
240+
218241
const name = createQueryName(tagId);
219242
const dbServerName =
220243
workerConfig?.dbServerName ?? dbServers[0]?.name ?? 'Query 1';
@@ -230,13 +253,13 @@ export async function createInteractiveConsoleQuery(
230253

231254
const jvmProfile =
232255
workerConfig?.jvmProfile ?? serverConfigValues.jvmProfileDefault;
256+
233257
const scriptLanguage =
234258
workerConfig?.scriptLanguage ??
235259
serverConfigValues.scriptSessionProviders?.find(
236260
p => p.toLocaleLowerCase() === consoleType
237261
) ??
238262
'Python';
239-
const workerKind = serverConfigValues.workerKinds?.[0]?.name;
240263

241264
const autoDelete = autoDeleteTimeoutMs > 0;
242265

@@ -256,21 +279,26 @@ export async function createInteractiveConsoleQuery(
256279
queueName,
257280
});
258281

282+
const { additionalMemory, classPaths, envVars } = workerConfig;
283+
259284
const draftQuery = new DraftQuery({
260-
isClientSide: true,
261-
draftOwner: owner,
262-
name,
263-
type,
264-
owner,
285+
additionalMemory: additionalMemory ?? 0,
265286
dbServerName,
287+
draftOwner: owner,
288+
envVars: envVars ?? '',
289+
extraClasspaths: classPaths ?? '',
266290
heapSize,
267-
scheduling,
291+
isClientSide: true,
268292
jvmArgs,
269293
jvmProfile,
294+
name,
295+
owner,
296+
scheduling,
270297
scriptLanguage,
271298
timeout,
299+
type,
272300
typeSpecificFields,
273-
workerKind,
301+
workerKind: engine,
274302
});
275303

276304
if (draftQuery.serial == null) {
@@ -461,6 +489,14 @@ export async function getWorkerInfoFromQuery(
461489
};
462490
}
463491

492+
/**
493+
* Determine if a given worker kind is a Community worker.
494+
* @returns True if the worker kind is a Community worker, false otherwise.
495+
*/
496+
export function isCommunityWorkerKind(workerKind?: WorkerKind): boolean {
497+
return workerKind?.protocols?.includes(PROTOCOL.COMMUNITY) ?? false;
498+
}
499+
464500
/**
465501
* Polyfill some things needed by the DHE jsapi. The need for this should go
466502
* away once DH-17942 is completed and the jsapi no longer relies on `window`

src/types/commonTypes.d.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,11 @@ export type ServerConnectionConfig =
130130
| URL;
131131

132132
export interface WorkerConfig {
133+
additionalMemory?: number;
134+
classPaths?: string;
133135
dbServerName?: string;
136+
engine?: string;
137+
envVars?: string;
134138
heapSize?: number;
135139
jvmArgs?: string;
136140
jvmProfile?: string;

src/util/uiUtils.spec.ts

Lines changed: 103 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
import * as vscode from 'vscode';
2-
import { describe, it, expect, vi } from 'vitest';
2+
import { beforeEach, describe, it, expect, vi } from 'vitest';
3+
import type { Username } from '@deephaven-enterprise/auth-nodejs';
34
import {
45
createConnectText,
56
ConnectionOption,
67
createConnectionOption,
78
updateConnectionStatusBarItem,
89
createConnectionQuickPickOptions,
910
createSeparatorPickItem,
11+
promptForCredentials,
1012
} from './uiUtils';
1113
import type {
1214
ConnectionState,
@@ -28,6 +30,10 @@ const groovyServerConfig: CoreConnectionConfig = {
2830
url: new URL('http://localhost:10001'),
2931
};
3032

33+
beforeEach(() => {
34+
vi.clearAllMocks();
35+
});
36+
3137
describe('createConnectionOption', () => {
3238
it.each([
3339
['DHC', pythonServerConfig],
@@ -138,3 +144,99 @@ describe('createSeparatorPickItem', () => {
138144
});
139145
});
140146
});
147+
148+
describe('promptForCredentials', () => {
149+
const title = 'mock.title';
150+
const username = 'mock.username' as Username;
151+
const token = 'mock.token';
152+
const operateAs = 'mock.operateAs';
153+
154+
it.each([
155+
[
156+
'password',
157+
{ title },
158+
[username, token],
159+
{ type: 'password', token, username },
160+
],
161+
['password / cancelled username', { title }, [undefined], undefined],
162+
['password / cancelled token', { title }, [username, undefined], undefined],
163+
[
164+
'password operateAs',
165+
{ title, showOperateAs: true },
166+
[username, token, operateAs],
167+
{ type: 'password', operateAs, token, username },
168+
],
169+
[
170+
'password operateAs / cancelled username',
171+
{ title, showOperateAs: true },
172+
[undefined],
173+
undefined,
174+
],
175+
[
176+
'password operateAs / cancelled token',
177+
{ title, showOperateAs: true },
178+
[username, undefined],
179+
undefined,
180+
],
181+
[
182+
'password operateAs / cancelled operateAs',
183+
{ title, showOperateAs: true },
184+
[username, token, undefined],
185+
undefined,
186+
],
187+
[
188+
'privateKey',
189+
{ title, privateKeyUserNames: [username] },
190+
[username],
191+
{ type: 'keyPair', username },
192+
],
193+
[
194+
'privateKey / cancelled username',
195+
{ title, privateKeyUserNames: [username] },
196+
[undefined],
197+
undefined,
198+
],
199+
[
200+
'privateKey operateAs',
201+
{
202+
title,
203+
privateKeyUserNames: [username],
204+
showOperateAs: true,
205+
},
206+
[username, operateAs],
207+
{ type: 'keyPair', username, operateAs },
208+
],
209+
[
210+
'privateKey operateAs / cancelled username',
211+
{
212+
title,
213+
privateKeyUserNames: [username],
214+
showOperateAs: true,
215+
},
216+
[undefined],
217+
undefined,
218+
],
219+
[
220+
'privateKey operateAs / cancelled operateAs',
221+
{
222+
title,
223+
privateKeyUserNames: [username],
224+
showOperateAs: true,
225+
},
226+
[username, undefined],
227+
undefined,
228+
],
229+
])(
230+
'should prompt for username, password, and operateAs: %s',
231+
async (_label, arg, promptResponses, expected) => {
232+
for (const promptResponse of promptResponses) {
233+
vi.mocked(vscode.window.showInputBox).mockResolvedValueOnce(
234+
promptResponse
235+
);
236+
}
237+
238+
const actual = await promptForCredentials(arg);
239+
expect(actual).toEqual(expected);
240+
}
241+
);
242+
});

src/util/uiUtils.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -168,31 +168,31 @@ export async function promptForAuthFlow(
168168
* @param privateKeyUserNames Optional list of private key user names. If provided,
169169
* the authentication method will be prompted to determine if user wants to use
170170
* one of these private keys or username/password.
171-
* @param showOperatesAs Whether to show the operate as prompt.
171+
* @param showOperateAs Whether to show the operate as prompt.
172172
*/
173173
export async function promptForCredentials(args: {
174174
title: string;
175175
userLoginPreferences?: UserLoginPreferences;
176176
privateKeyUserNames?: undefined | [];
177-
showOperatesAs?: boolean;
177+
showOperateAs?: boolean;
178178
}): Promise<PasswordCredentials | undefined>;
179179
export async function promptForCredentials(args: {
180180
title: string;
181181
userLoginPreferences?: UserLoginPreferences;
182182
privateKeyUserNames?: Username[];
183-
showOperatesAs?: boolean;
183+
showOperateAs?: boolean;
184184
}): Promise<LoginPromptCredentials | undefined>;
185185
export async function promptForCredentials(args: {
186186
title: string;
187187
userLoginPreferences?: UserLoginPreferences;
188188
privateKeyUserNames?: Username[];
189-
showOperatesAs?: boolean;
189+
showOperateAs?: boolean;
190190
}): Promise<LoginPromptCredentials | undefined> {
191191
const {
192192
title,
193193
userLoginPreferences,
194194
privateKeyUserNames = [],
195-
showOperatesAs,
195+
showOperateAs,
196196
} = args;
197197

198198
const username = await promptForUsername(
@@ -220,10 +220,10 @@ export async function promptForCredentials(args: {
220220
}
221221

222222
// Operate As
223-
if (showOperatesAs) {
223+
if (showOperateAs) {
224224
const defaultValue = username as unknown as OperateAsUsername | undefined;
225225

226-
const operateAs = await promptForOperateAs(
226+
operateAs = await promptForOperateAs(
227227
title,
228228
userLoginPreferences?.operateAsUser[username] ?? defaultValue
229229
);

0 commit comments

Comments
 (0)