feat: New MCP Tools for Persistent Queries #139
Conversation
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
…guration options Renamed make_temporary_config() to make_pq_config() to better reflect its purpose of creating persistent query configurations. Added support for script content (inline or file-based), programming language selection (Python/Groovy), configuration types, scheduling, JVM profiles, Python virtual environments, initialization timeouts, and restart user controls. Updated method documentation and references throughout the codebase.
Refactored persistent query configuration logic by extracting parameter application into a new `_apply_pq_config_parameters` method. This improves code organization and maintainability by separating the protobuf config modification logic from the main `make_pq_config` method. Also fixed formatting for multi-line error messages and improved docstring clarity.
Enhanced type hints throughout CorePlusControllerClient by replacing generic types with specific ones and making optional parameters explicit. Updated docstrings to use proper parameter type annotations. Changed make_pq_config to accept None for programming_language, configuration_type, and enabled parameters to preserve controller defaults. Fixed script_body to use scriptCode field in protobuf config. Updated _apply_pq_config_parameters
Standardized code formatting by removing trailing whitespace throughout the codebase. Enhanced type hints in multiple functions by replacing generic types with specific ones (e.g., `int` → `CorePlusQuerySerial`, `Any` for protobuf objects). Improved docstrings with proper parameter type annotations and more concise descriptions. Split long f-string error messages across multiple lines for better readability. Updated import statements to include
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request introduces comprehensive support for Persistent Query (PQ) management in Deephaven Enterprise, standardizes terminology to use "Core+" instead of "CorePlus", and adds developer best practices. The changes enable full lifecycle management of persistent queries with new MCP tools, automatic controller subscription for PQ operations, and extensive test coverage.
Key Changes:
- Added 7 new MCP tools for PQ management (list, details, create, delete, start, stop, restart, name_to_id)
- Implemented automatic subscription to persistent query state in controller clients during factory initialization
- Standardized terminology from "CorePlus"/"Enterprise Edition" to "Core+" throughout codebase
- Added comprehensive test coverage (~2600 new test lines) for PQ operations and protobuf formatting
- Enhanced error messages to include exception type information for better debugging
- Added new programming best practices (file-to-test mapping, avoiding Any/hasattr/getattr)
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/resource_manager/test_registry_combined.py | Added subscription mocks to controller client tests for new automatic subscription behavior |
| tests/mcp_systems_server/test_mcp_systems_server__mcp.py | Added 2600+ lines of comprehensive PQ management tests and protobuf formatting helper tests |
| tests/client/test_controller_client.py | Added subscription tests, expanded make_pq_config tests with all parameters, fixed mock setup |
| src/deephaven_mcp/resource_manager/_registry_combined.py | Added controller subscription call, updated terminology to Core+, enhanced docstrings |
| src/deephaven_mcp/mcp_systems_server/_mcp.py | Added 8 PQ management tools, formatting helpers, error message improvements |
| src/deephaven_mcp/client/_session_factory.py | Added automatic subscribe() calls during factory initialization, enhanced docstrings |
| src/deephaven_mcp/client/_controller_client.py | Added subscribe() method with idempotency, replaced make_temporary_config with make_pq_config, subscription guards |
| docs/DEVELOPER_GUIDE.md | Added comprehensive PQ management tool documentation with examples |
| README.md | Updated feature list to include PQ management capabilities |
| .windsurf/rules/programming-practices.md | Added 3 new programming practices for test organization and type safety |
| with patch.object( | ||
| controller_client_mod, "CorePlusQueryConfig", autospec=True | ||
| ) as mock_cfg: | ||
| result = await coreplus_controller_client.make_pq_config( |
There was a problem hiding this comment.
Variable result is not used.
Suggested change
| result = await coreplus_controller_client.make_pq_config( | |
| await coreplus_controller_client.make_pq_config( |
| with patch.object( | ||
| controller_client_mod, "CorePlusQueryConfig", autospec=True | ||
| ) as mock_cfg: | ||
| result = await coreplus_controller_client.make_pq_config( |
There was a problem hiding this comment.
Variable result is not used.
| controller_client_mod, "CorePlusQueryConfig", autospec=True | ||
| ) as mock_cfg: | ||
| # Call with minimal parameters - None defaults should NOT override | ||
| result = await coreplus_controller_client.make_pq_config( |
There was a problem hiding this comment.
Variable result is not used.
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.
This pull request introduces comprehensive support for Persistent Query (PQ) management in Deephaven Enterprise, updates documentation and API references to clarify Core+ terminology, and improves the developer and user experience with detailed docstrings and best practice guidelines. The most significant changes are grouped below:
Persistent Query (PQ) Management:
README.mdanddocs/DEVELOPER_GUIDE.md. This includes new API commands (pq_list,pq_details,pq_create,pq_start,pq_stop,pq_restart,pq_delete) and workflow examples. [1] [2] [3]CorePlusSessionFactoryandCombinedSessionRegistryto ensure that controller clients automatically subscribe to persistent query state upon creation, enabling real-time PQ management. [1] [2] [3]Terminology and Documentation Updates:
Developer Best Practices:
Anyas a type hint and the use ofhasattr/getattrwithout justification.These changes collectively improve the robustness, usability, and maintainability of the codebase, especially for teams working with enterprise persistent queries and session management.