Skip to content

Commit ceeb6c9

Browse files
authored
feat: DH-21221: Remote python controller imports (#315)
DH-21221: Remote python controller imports ## Test Setup - Clone `git@github.com:bmingles/deephaven-controller-script-test.git` - Checkout `local` branch - Open `.vscode/settings.json` - There should be a config for `https://bmingles-remote-file-source2.int.illumon.com:8123` BHS - Create a connection to the server - Open OUTPUT -> Deephaven panel - Close VS Code - Run this branch in debugger - When new window opens, open the folder of the controller script repo - Connect to `https://bmingles-remote-file-source2.int.illumon.com:8123` ## Tests ### Default Prefix Test - Run `src/main/python/default_prefix_test.py` - OUTPUT -> Deephaven panel should show ``` STDOUT Controller sourced package1.subpackage1.testmodule1 STDOUT Controller sourced package2.subpackage1.testmodule1 STDOUT Controller sourced package2.subpackage1.testmodule2 ``` - Add `src/main/python/package1` folder as python remote file source - Run script again. Should see ``` STDOUT Local Sourced package1.subpackage1.testmodule1 STDOUT Controller sourced package2.subpackage1.testmodule1 STDOUT Controller sourced package2.subpackage1.testmodule2 ``` - Remove the folder as remote file source and run again. Should see the all controller sourced output again - Add `src/main/python/package2` as remote source and run again. Should see ``` STDOUT Controller sourced package1.subpackage1.testmodule1 STDOUT Local Sourced package2.subpackage1.testmodule1 STDOUT Local Sourced package2.subpackage1.testmodule2 ``` - Remove folder run again should show back to all controller sources - Try adding package1 and package2 as sources, run should see `Local,Local,Local` - Remove package1, run should see `Controller,Local,Local` - Remove package2, run should be back to `Controller,Controller,Controller` ### Custom Prefix Test - Run `src/main/python/custom_prefix_test.py`, run should see `Controller,Controller,Controller` - Add package12 run should still see `Controller,Controller,Controller` (this is because `src/main/python/package1/subpackage1/testmodule1.py` has a different meta_import()) - Add `"deephaven.importPrefixes": ["custom_controller", "controller"]` to `.vscode/settings.json` - Run script again should show `Controller,Local,Local`
1 parent d50e8b3 commit ceeb6c9

19 files changed

Lines changed: 947 additions & 17 deletions

__mocks__/vscode.ts

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -129,18 +129,28 @@ export class DiagnosticCollection
129129
}
130130

131131
export class EventEmitter<T> {
132-
listeners = new Set<(...args: any[]) => void>();
133-
event = (listener: (e: T) => any) => {
134-
this.listeners.add(listener);
135-
return () => {
136-
this.listeners.delete(listener);
137-
};
138-
};
132+
private listeners = new Set<(data: T) => any>();
133+
134+
event = vi
135+
.fn()
136+
.mockName('event')
137+
.mockImplementation((listener: (e: T) => any) => {
138+
this.listeners.add(listener);
139+
return {
140+
dispose: vi
141+
.fn()
142+
.mockName('dispose')
143+
.mockImplementation(() => {
144+
this.listeners.delete(listener);
145+
}),
146+
};
147+
});
148+
139149
fire = vi
140150
.fn()
141151
.mockName('fire')
142-
.mockImplementation((event: T) => {
143-
this.listeners.forEach(listener => listener(event));
152+
.mockImplementation((data: T): void => {
153+
this.listeners.forEach(listener => listener(data));
144154
});
145155
}
146156

@@ -354,7 +364,10 @@ export class Uri {
354364
static joinPath = vi
355365
.fn()
356366
.mockName('joinPath')
357-
.mockImplementation((...args) => Uri.parse(args.join('/')));
367+
.mockImplementation((...args) => {
368+
const filteredArgs = args.filter(a => a.toString().length > 0);
369+
return Uri.parse(filteredArgs.join('/'));
370+
});
358371

359372
static parse = vi
360373
.fn()

docs/python-remote-file-sourcing.md

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,3 +156,74 @@ def dashboard_content(table):
156156
2. In your VS Code workspace, use the Deephaven extension to run `main.py`. The imports will resolve because the `stock_ticker` folder is registered as a remote file source.
157157

158158
![Run main.py](assets/run-main-py.gif)
159+
160+
## Controller Import Prefix Support (Enterprise)
161+
162+
**Deephaven Enterprise** uses a controller import registration mechanism (`meta_import()`) that requires modules to be importable under a prefixed name (e.g., `controller.mymodule`) for the server to find them. The VS Code extension automatically detects this pattern in your Python code and registers both the unprefixed and prefixed names with the server for each folder marked as a remote file source, allowing prefixed imports to be sourced by the extension.
163+
164+
### Auto-Detection
165+
166+
When you run Python code, the extension scans for `meta_import()` calls and infers the prefix to use. No extra setup is required — if your code already calls `meta_import()`, the extension picks it up automatically.
167+
168+
**With default prefix (`controller`):**
169+
170+
```python
171+
import deephaven_enterprise.controller_import
172+
173+
deephaven_enterprise.controller_import.meta_import()
174+
```
175+
176+
**With a custom prefix:**
177+
178+
```python
179+
import deephaven_enterprise.controller_import
180+
181+
deephaven_enterprise.controller_import.meta_import("myprefix")
182+
```
183+
184+
**From-import style:**
185+
186+
```python
187+
from deephaven_enterprise.controller_import import meta_import
188+
189+
meta_import("custom")
190+
```
191+
192+
### Behavior
193+
194+
- For each folder marked as a remote file source, both the unprefixed and prefixed module names are registered with the Deephaven server.
195+
- Example: If you mark a folder called `mymodule` and a prefix of `controller` is detected, the server will receive both `mymodule` and `controller.mymodule` as importable names for that folder.
196+
- Without a detected or configured prefix, only the unprefixed name (`mymodule`) is registered for each marked folder.
197+
- Prefixes are **updated when you run Python code**: running a full file replaces any previous prefixes; running a snippet updates prefixes only if a `meta_import()` call is found in that snippet.
198+
199+
### Manual Override
200+
201+
You can set one or more prefixes explicitly in your VS Code settings:
202+
203+
```json
204+
"deephaven.importPrefixes": ["controller"]
205+
```
206+
207+
When set, this array is used as the source of truth and auto-detection is skipped entirely. Multiple prefixes can be provided if needed:
208+
209+
```json
210+
"deephaven.importPrefixes": ["controller", "custom"]
211+
```
212+
213+
The main reasons to set this manually are:
214+
215+
- **Unrecognized imports** — the auto-detection doesn't recognize all possible import patterns such as aliasing:
216+
217+
```python
218+
import deephaven_enterprise.controller_import as ci
219+
220+
ci.meta_import()
221+
222+
from deephaven_enterprise.controller_import import meta_import as m
223+
224+
m()
225+
```
226+
227+
- **`meta_import()` in a dependency** — auto-detection only scans the code being run directly, not modules it imports. If the registration happens inside a dependency rather than the script itself, the prefix will not be detected.
228+
229+
If either case applies, set `deephaven.importPrefixes` manually.

package.json

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,17 @@
160160
},
161161
"default": []
162162
},
163+
"deephaven.importPrefixes": {
164+
"type": [
165+
"array",
166+
"null"
167+
],
168+
"default": null,
169+
"items": {
170+
"type": "string"
171+
},
172+
"markdownDescription": "Optional controller import prefixes for remote file sourcing. When set, overrides automatic detection from `meta_import()` calls in Python code. Defaults to automatic detection if not specified."
173+
},
163174
"deephaven.mcp.enabled": {
164175
"type": "boolean",
165176
"default": false,

src/common/constants.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ export const CONFIG_KEY = {
1616
root: 'deephaven',
1717
coreServers: 'coreServers',
1818
enterpriseServers: 'enterpriseServers',
19+
importPrefixes: 'importPrefixes',
1920
mcpAutoUpdateConfig: 'mcp.autoUpdateConfig',
2021
mcpDocsEnabled: 'mcp.docsEnabled',
2122
mcpEnabled: 'mcp.enabled',

src/services/ConfigService.spec.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,3 +353,39 @@ describe('updateWindsurfMcpConfig', () => {
353353
expect(getEnsuredContent).toHaveBeenCalledWith(windowsConfigUri, '{}\n');
354354
});
355355
});
356+
357+
describe('getImportPrefixes', () => {
358+
it.each([
359+
['not configured', undefined, undefined],
360+
['empty array', [], []],
361+
['single valid prefix', ['controller'], ['controller']],
362+
['multiple valid prefixes', ['controller', '_my_prefix'], ['controller', '_my_prefix']],
363+
])('should return without errors when %s', (_label, given, expected) => {
364+
configMap.set(CONFIG_KEY.importPrefixes, given);
365+
366+
const result = ConfigService.getImportPrefixes();
367+
368+
expect(result).toEqual(expected);
369+
expect(vscode.window.showErrorMessage).not.toHaveBeenCalled();
370+
});
371+
372+
it.each([
373+
['one invalid entry (empty string)', [''], [], ['']],
374+
['one invalid entry (starts with digit)', ['1controller'], [], ['1controller']],
375+
['one invalid entry (contains hyphen)', ['my-prefix'], [], ['my-prefix']],
376+
['one invalid entry (dotted name)', ['my.prefix'], [], ['my.prefix']],
377+
['mixed valid and invalid entries', ['valid', '1bad', 'also_valid', 'my-prefix'], ['valid', 'also_valid'], ['1bad', 'my-prefix']],
378+
])('should filter invalid entries and show errors when %s', (_label, given, expectedResult, expectedInvalid) => {
379+
configMap.set(CONFIG_KEY.importPrefixes, given);
380+
381+
const result = ConfigService.getImportPrefixes();
382+
383+
expect(result).toEqual(expectedResult);
384+
expect(vscode.window.showErrorMessage).toHaveBeenCalledTimes(expectedInvalid.length);
385+
for (const invalid of expectedInvalid) {
386+
expect(vscode.window.showErrorMessage).toHaveBeenCalledWith(
387+
expect.stringContaining(`'${invalid}' is not a valid import prefix name`)
388+
);
389+
}
390+
});
391+
});

src/services/ConfigService.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,28 @@ function hasValidURL({ url }: { url: string }): boolean {
7878
}
7979
}
8080

81+
// ASCII subset of Python identifier rules (PEP 3131 / py3 lexical spec allows Unicode,
82+
// but controller prefix names are expected to be ASCII in practice). A prefix is a
83+
// single identifier, not a dotted module path.
84+
const VALID_PYTHON_IDENTIFIER_RE = /^[A-Za-z_][A-Za-z0-9_]*$/;
85+
86+
function getImportPrefixes(): string[] | undefined {
87+
const config = getConfig().get<string[] | null>(CONFIG_KEY.importPrefixes);
88+
if (config == null) {
89+
return undefined;
90+
}
91+
92+
return config.filter(prefix => {
93+
if (VALID_PYTHON_IDENTIFIER_RE.test(prefix)) {
94+
return true;
95+
}
96+
vscode.window.showErrorMessage(
97+
`Invalid 'deephaven.importPrefixes' setting: '${prefix}' is not a valid import prefix name. It will be ignored.`
98+
);
99+
return false;
100+
});
101+
}
102+
81103
async function toggleMcp(enable?: boolean): Promise<void> {
82104
const currentState = isMcpEnabled();
83105
const targetState = enable ?? !currentState;
@@ -233,6 +255,7 @@ export async function updateWindsurfMcpConfig(
233255
export const ConfigService: IConfigService = {
234256
getCoreServers,
235257
getEnterpriseServers,
258+
getImportPrefixes,
236259
isElectronFetchEnabled,
237260
isMcpDocsEnabled,
238261
isMcpEnabled,

src/services/DhcService.spec.ts

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
import * as vscode from 'vscode';
2+
import type { dh as DhcType } from '@deephaven/jsapi-types';
3+
import { beforeEach, describe, expect, it, vi } from 'vitest';
4+
import { DhcService } from './DhcService';
5+
import { ConfigService } from './ConfigService';
6+
import type { RemoteFileSourceService } from './RemoteFileSourceService';
7+
import type { UniqueID } from '../types';
8+
9+
vi.mock('vscode');
10+
11+
vi.mock('./ConfigService', () => {
12+
const mockConfigService = {
13+
getImportPrefixes: vi.fn(),
14+
};
15+
// eslint-disable-next-line @typescript-eslint/naming-convention
16+
return { ConfigService: mockConfigService };
17+
});
18+
19+
/** Build a minimal DhcService instance that has a session already set up. */
20+
function createTestDhcService({
21+
pythonRemoteFileSourcePlugin,
22+
setControllerImportPrefixes = vi.fn(),
23+
setPythonServerExecutionContext = vi.fn().mockResolvedValue(undefined),
24+
sessionRunCode = vi.fn().mockResolvedValue({
25+
error: '',
26+
changes: { created: [], updated: [], removed: [] },
27+
}),
28+
}: {
29+
pythonRemoteFileSourcePlugin?: DhcType.Widget | null;
30+
setControllerImportPrefixes?: ReturnType<typeof vi.fn>;
31+
setPythonServerExecutionContext?: ReturnType<typeof vi.fn>;
32+
sessionRunCode?: ReturnType<typeof vi.fn>;
33+
} = {}): DhcService {
34+
const mockSession = {
35+
runCode: sessionRunCode,
36+
} as unknown as DhcType.IdeSession;
37+
38+
const mockCn = {
39+
getConsoleTypes: vi.fn().mockResolvedValue(['python']),
40+
} as unknown as DhcType.IdeConnection;
41+
42+
const mockRemoteFileSourceService = {
43+
setControllerImportPrefixes,
44+
setPythonServerExecutionContext,
45+
} as unknown as RemoteFileSourceService;
46+
47+
const mockDiagnosticsCollection = {
48+
set: vi.fn(),
49+
clear: vi.fn(),
50+
} as unknown as vscode.DiagnosticCollection;
51+
52+
const service = Object.assign(Object.create(DhcService.prototype), {
53+
session: mockSession,
54+
cn: mockCn,
55+
cnId: 'test-cn-id' as UniqueID,
56+
pythonRemoteFileSourcePlugin:
57+
pythonRemoteFileSourcePlugin !== undefined
58+
? pythonRemoteFileSourcePlugin
59+
: ({} as DhcType.Widget),
60+
groovyRemoteFileSourcePluginService: null,
61+
remoteFileSourceService: mockRemoteFileSourceService,
62+
diagnosticsCollection: mockDiagnosticsCollection,
63+
groovyDiagnosticsCollection: mockDiagnosticsCollection,
64+
outputChannel: {
65+
appendLine: vi.fn(),
66+
show: vi.fn(),
67+
} as unknown as vscode.OutputChannel,
68+
toaster: { error: vi.fn(), info: vi.fn() },
69+
_isRunningCode: false,
70+
_onDidChangeRunningCodeStatus: { fire: vi.fn() },
71+
disposables: { add: vi.fn() },
72+
serverUrl: new URL('http://localhost:10000/'),
73+
});
74+
75+
return service;
76+
}
77+
78+
function mockTextDoc(codeText: string): vscode.TextDocument {
79+
return {
80+
uri: vscode.Uri.file('/path/to/file.py'),
81+
getText: vi.fn().mockReturnValue(codeText),
82+
} as unknown as vscode.TextDocument;
83+
}
84+
85+
describe('DhcService.runCode – importPrefixes setting', () => {
86+
const mockSetControllerImportPrefixes = vi.fn();
87+
88+
beforeEach(() => {
89+
vi.clearAllMocks();
90+
});
91+
92+
it.each([
93+
{
94+
label: 'uses setting prefixes when configured',
95+
configPrefixes: ['myPrefix'],
96+
input: 'x = 1',
97+
expected: new Set(['myPrefix']),
98+
},
99+
{
100+
label: 'uses all prefixes when multiple configured',
101+
configPrefixes: ['prefix1', 'prefix2'],
102+
input: 'x = 1',
103+
expected: new Set(['prefix1', 'prefix2']),
104+
},
105+
{
106+
label: 'falls back to extraction when undefined and code has prefixes',
107+
configPrefixes: undefined,
108+
input: 'deephaven_enterprise.controller_import.meta_import("myPrefix")\n',
109+
expected: new Set(['myPrefix']),
110+
},
111+
{
112+
label: 'skips call when undefined and no prefixes in snippet',
113+
configPrefixes: undefined,
114+
input: 'x = 1',
115+
expected: null,
116+
},
117+
{
118+
label: 'calls with empty set for full file runs even when no prefixes found',
119+
configPrefixes: undefined,
120+
input: mockTextDoc('x = 1'),
121+
expected: new Set(),
122+
},
123+
{
124+
label: 'setting takes precedence over meta_import in code',
125+
configPrefixes: ['forced'],
126+
input: 'deephaven_enterprise.controller_import.meta_import("otherPrefix")\n',
127+
expected: new Set(['forced']),
128+
},
129+
{
130+
label: 'setting takes precedence over meta_import in text doc',
131+
configPrefixes: ['forced'],
132+
input: mockTextDoc(
133+
'deephaven_enterprise.controller_import.meta_import("otherPrefix")\n'
134+
),
135+
expected: new Set(['forced']),
136+
},
137+
])('$label', async ({ configPrefixes, input, expected }) => {
138+
vi.mocked(ConfigService.getImportPrefixes).mockReturnValue(configPrefixes);
139+
140+
const service = createTestDhcService({
141+
setControllerImportPrefixes: mockSetControllerImportPrefixes,
142+
});
143+
144+
await service.runCode(input, 'python');
145+
146+
if (expected == null) {
147+
expect(mockSetControllerImportPrefixes).not.toHaveBeenCalled();
148+
} else {
149+
expect(mockSetControllerImportPrefixes).toHaveBeenCalledWith(expected);
150+
}
151+
});
152+
});

0 commit comments

Comments
 (0)