feat: DH-21221: Remote python controller imports#315
Draft
bmingles wants to merge 8 commits intoDH-20578_groovy-remote-file-sourcefrom
Draft
feat: DH-21221: Remote python controller imports#315bmingles wants to merge 8 commits intoDH-20578_groovy-remote-file-sourcefrom
bmingles wants to merge 8 commits intoDH-20578_groovy-remote-file-sourcefrom
Conversation
22f7c02 to
94eb4ff
Compare
…ollerImportScanner service to detect controller import configuration in workspace Python files. Currently, the extension sends only unprefixed module names (e.g. 'mymodule') to the Deephaven server. This task creates a new service that scans .py files for deephaven_enterprise.controller_import.meta_import() usage and extracts the prefix argument (defaulting to 'controller' if not provided). The service should: 1) Extend DisposableBase, 2) Accept FilteredWorkspace<PythonModuleFullname> in constructor, 3) Subscribe to workspace.onDidUpdate() to trigger re-scanning, 4) Use regex to detect two patterns: 'import deephaven_enterprise.controller_import' + 'meta_import()' calls, and 'from deephaven_enterprise.controller_import import meta_import' + 'meta_import()' calls, 5) Extract string argument from meta_import(arg) or default to 'controller', 6) Expose getControllerPrefix(): string | null method, 7) Fire onDidUpdatePrefix event when prefix changes, 8) Implement debouncing (500ms) to avoid excessive re-scanning, 9) Stop scanning after first match found (first-match-wins strategy). Create file at src/services/PythonControllerImportScanner.ts Created PythonControllerImportScanner service at src/services/PythonControllerImportScanner.ts. Extends DisposableBase, accepts FilteredWorkspace<PythonModuleFullname>, subscribes to workspace updates, implements regex-based scanning for both direct and from-import patterns, defaults prefix to 'controller', exposes getControllerPrefix() and onDidUpdatePrefix event, debounces with 500ms, stops after first match. (#DH-21221)
…ve unit tests for PythonControllerImportScanner service. The scanner needs tests to verify it correctly detects controller import configurations in Python code. Create src/services/PythonControllerImportScanner.spec.ts with Vitest tests covering: 1) Detects 'import deephaven_enterprise.controller_import' followed by 'meta_import()' (default prefix='controller'), 2) Detects 'meta_import("customprefix")' and extracts 'customprefix', 3) Detects 'from deephaven_enterprise.controller_import import meta_import' followed by 'meta_import()', 4) Returns null when no configuration found, 5) First match wins when multiple files have different configs, 6) Fires onDidUpdatePrefix event when prefix changes, 7) Does not fire event when prefix stays the same, 8) Handles invalid/malformed Python gracefully, 9) Debounces multiple rapid workspace updates. Follow existing test patterns from FilteredWorkspace.spec.ts (mock vscode module, use vi.mock, beforeEach for cleanup, parameterized tests with describe.each for pattern variations). Use AGENTS.md guidelines: always use 'npx vitest run' for test execution, never watch mode.
Created src/services/PythonControllerImportScanner.spec.ts with 24 tests covering: both import patterns, default/custom prefix extraction, null return, first-match-wins, event firing, no-event-on-same-prefix, debouncing, error handling (#DH-21221)
…ntrollerImportScanner into RemoteFileSourceService to send prefixed module names to the Deephaven server. Currently, RemoteFileSourceService.getPythonTopLevelModuleNames() returns only unprefixed module names (e.g. {'mymodule'}). After this task, it should return both unprefixed and prefixed names when a controller import is configured (e.g. {'mymodule', 'controller.mymodule'}). Changes needed in src/services/RemoteFileSourceService.ts: 1) Add _controllerImportScanner: PythonControllerImportScanner parameter to constructor, 2) Store scanner as private field, 3) Subscribe to scanner.onDidUpdatePrefix() and fire _onDidUpdatePythonModuleMeta when prefix changes, 4) Update getPythonTopLevelModuleNames() to: a) Get prefix via scanner.getControllerPrefix(), b) For each marked folder module name, add unprefixed version to set, c) If prefix is not null, also add prefixed version (e.g. 'controller.mymodule') to set, 5) Add proper disposal of scanner subscription. See .tasks/controller-import-prefix-support.md for detailed implementation example. Also update src/services/index.ts to export PythonControllerImportScanner.
Integrated PythonControllerImportScanner into RemoteFileSourceService: added scanner as constructor parameter, subscribed to onDidUpdatePrefix to fire _onDidUpdatePythonModuleMeta, updated getPythonTopLevelModuleNames() to return both prefixed and unprefixed names, exported PythonControllerImportScanner from index.ts, and updated ExtensionController.ts to instantiate and wire up the scanner. (#DH-21221)
… RemoteFileSourceService integration with PythonControllerImportScanner. Currently, RemoteFileSourceService has no test file. Create src/services/RemoteFileSourceService.spec.ts to test the getPythonTopLevelModuleNames() method with and without controller prefix configuration. Test scenarios: 1) No prefix configured (scanner returns null) - should return only unprefixed names like {'mymodule', 'othermodule'}, 2) Default prefix configured (scanner returns 'controller') - should return both unprefixed and prefixed like {'mymodule', 'controller.mymodule', 'othermodule', 'controller.othermodule'}, 3) Custom prefix configured (scanner returns 'custom') - should return {'mymodule', 'custom.mymodule'}, 4) Prefix changes trigger _onDidUpdatePythonModuleMeta event. Mock dependencies: FilteredWorkspace (groovy and python), PythonControllerImportScanner. Use patterns from FilteredWorkspace.spec.ts for mocking and test structure. Follow AGENTS.md: use 'npx vitest run' for test execution.
Created src/services/RemoteFileSourceService.spec.ts with 13 tests covering getPythonTopLevelModuleNames() with no prefix (returns only unprefixed), default 'controller' prefix, custom prefix, empty folders, and event firing when scanner prefix or python workspace changes. All tests pass. (#DH-21221)
…ntation to explain the controller import prefix feature. The docs/python-remote-file-sourcing.md file currently explains the basic remote file source feature but doesn't cover controller-prefixed imports. Add a new section titled 'Controller Import Prefix Support (Enterprise)' after the 'Example: Using a Local Package as a Remote File Source' section. The section should explain: 1) What controller import prefix support is (for Deephaven Enterprise users), 2) How to configure it in Python code (show both default and custom prefix examples), 3) What happens when configured (both prefixed and unprefixed module names are sent to server), 4) Supported Python import patterns (the two patterns that the regex detects), 5) Limitations (import aliases not supported, only one configuration per workspace), 6) That configuration is auto-detected when .py files are saved. Use clear code examples. See .tasks/controller-import-prefix-support.md section 'Documentation > User Documentation' for detailed content template. Added 'Controller Import Prefix Support (Enterprise)' section to docs/python-remote-file-sourcing.md covering feature purpose, configuration examples (default and custom prefix), behavior, supported import patterns, and limitations. (#DH-21221)
…functionality and ensure all tests pass. This is the final verification task to ensure the controller import prefix feature works correctly. Steps: 1) Run all unit tests with 'npx vitest run' to verify no regressions, 2) Run TypeScript compiler with 'npx tsc --noEmit' to verify no type errors, 3) Manually test the feature by: a) Creating a test workspace with a Python file containing 'import deephaven_enterprise.controller_import' and 'deephaven_enterprise.controller_import.meta_import()', b) Marking a folder as remote file source, c) Verifying the extension sends both prefixed and unprefixed module names to the server (check logs or execution context script), 4) Test with custom prefix by changing to 'meta_import("custom")', 5) Test that removing the meta_import configuration reverts to unprefixed-only behavior. Create a summary of test results and any issues found. If all tests pass and manual testing succeeds, the feature is ready for review.
Fixed missing onDidChange in vscode mock (__mocks__/vscode.ts) which was causing 12 FilteredWorkspace tests to fail. All 756 unit tests now pass and TypeScript compiles without errors. (#DH-21221)
8f774d5 to
9f0df4b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DH-21221: Remote python controller imports