MCP: Bring Databricks provider to the state of the Rust Version #3941
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
fjakobs
requested review from
a team,
lennartkats-db and
pietern
as code owners
fjakobs
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
fjakobs
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
fjakobs
changed the title
fjakobs
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
fjakobs
commented
Nov 18, 2025
experimental/apps-mcp/docs/porting-plan/phase-2-databricks-integration.md
Outdated
Show resolved
Hide resolved
fjakobs
commented
Nov 18, 2025
fjakobs
commented
Nov 18, 2025
fjakobs
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Collaborator
|
Commit: 495acc4
13 failing tests:
Top 50 slowest tests (at least 2 minutes):
|
arsenyinfo
reviewed
Nov 18, 2025
lennartkats-db
requested changes
Nov 18, 2025
Contributor
lennartkats-db
left a comment
lennartkats-db
left a comment
There was a problem hiding this comment.
Could you please add a human-authored or human-curated PR description? And indicate whether this is ready to merge? Current description suggests it is not. Otherwise, please review comments inline
keugenek
reviewed
Nov 18, 2025
Contributor
keugenek
left a comment
keugenek
left a comment
There was a problem hiding this comment.
I looked through the code - looks almost completely to be replaced by SDK layer in future - is my understanding agrees with yours @fjakobs @MarioCadenas ?
fjakobs
force-pushed
the
mcp/pagination-support-list-tables
branch
from
b46024e to
d0b86de
Compare
fjakobs
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
fjakobs
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
fjakobs
changed the title
Contributor
Author
|
@keugenek chances are that some/most of this code will be replaced by direct CLI calls. For now I want this so we don't regress in the go version compared to rust |
fjakobs
force-pushed
the
mcp/pagination-support-list-tables
branch
from
afe2ef3 to
4eb3210
Compare
fjakobs
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
fjakobs
commented
Nov 19, 2025
fjakobs
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
arsenyinfo
approved these changes
Nov 19, 2025
| ) | ||
|
|
||
| // ============================================================================ | ||
| // Helper Functions |
Contributor
There was a problem hiding this comment.
do they really belong to provider implementation?
lennartkats-db
approved these changes
Nov 19, 2025
| }), | ||
| ) | ||
|
|
||
| log.Infof(p.ctx, "Registered workspace tools: count=%d", 6) |
Contributor
There was a problem hiding this comment.
Logline was not helpful?
fjakobs
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Implements Phase 2.1 from the porting plan to add pagination capabilities for handling large catalogs with many tables efficiently. Changes: - Add PageSize, PageToken, and Filter parameters to ListTablesArgs - Add NextPageToken and TotalCount fields to ListTablesResult - Implement pagination logic using direct API calls to access page tokens - Add matchFilter() utility function with wildcard support - Update MCP tool registration to expose new pagination parameters - Enhance result formatting to display pagination information Features: - Backward compatible (existing calls work with default page size: 100) - Supports filtering with wildcard patterns (e.g., "sales*", "*_staging") - Maximum page size: 1000 tables - Returns next page token when more results are available 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
This commit implements enhanced SQL query execution capabilities with result size limiting, timeout handling, and execution time tracking, as specified in Phase 2.2 of the porting plan. ## Changes ### Enhanced SQL Query Execution (sql.go) - Added `ExecuteQueryArgs` struct with optional parameters: - `WarehouseID`: Override default warehouse from config - `MaxRows`: Limit result rows (default: 1000, max: 10000) - `Timeout`: Query timeout in seconds (default: 60) - Created `ExecuteQueryResult` struct with enhanced output: - `Columns`, `Rows`: Query results - `RowCount`: Number of rows returned - `Truncated`: Indicates if results were limited - `ExecutionTime`: Query execution time in seconds - Implemented context-based timeout handling - Added automatic result truncation with configurable limits - Enhanced error messages for query failures ### Updated Formatting (format.go) - Modified `formatQueryResult()` to display execution time - Added truncation indicators when results are limited - Improved output formatting for large result sets - Modernized code using Go 1.24 `min()` builtin ### Updated Provider Registration (provider.go) - Added new optional parameters to `databricks_execute_query` tool - Enhanced tool description with execution time and limits info - Updated handler to use new `ExecuteQueryArgs` structure ### Updated Table Operations (tables.go) - Updated `DescribeTable()` to use new `ExecuteQuery()` signature - Enhanced result conversion to handle new `ExecuteQueryResult` - Improved type handling for count queries (int64/float64) ### Comprehensive Unit Tests - Added `format_test.go` with 31 test cases covering: - Catalog, schema, table, and query result formatting - Edge cases: empty results, null values, truncation - Large result sets and pagination scenarios - Added `catalogs_test.go` with 14 test cases for: - Wildcard pattern matching (`matchFilter`) - Complex filter patterns and edge cases - All 45 tests pass with 31.8% code coverage - Tests follow project conventions and style ### Documentation - Added Phase 2.2 porting plan documentation - Updated success criteria checklist - Documented implementation approach and testing strategy ## Testing - ✅ All unit tests pass (45 test cases) - ✅ No regressions in existing tests - ✅ Code builds successfully - ✅ Linter passes with no issues - ✅ Code properly formatted ## Backward Compatibility All changes are backward compatible. Existing calls to `databricks_execute_query` continue to work with default values, while new parameters provide enhanced control. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Replace deprecated `w.Config.NewApiClient()` with the recommended approach using `config.HTTPClientConfigFromConfig()` and `httpclient.NewApiClient()`. This fixes the staticcheck warning: SA1019: w.Config.NewApiClient is deprecated: use [HTTPClientConfigFromConfig] with [httpclient.NewApiClient] Changes: - Added import for "github.com/databricks/databricks-sdk-go/config" - Updated ListTables() to use config.HTTPClientConfigFromConfig() - Updated error message to reflect the new function 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Remove experimental/apps-mcp/docs/porting-plan/phase-2-databricks-integration.md - Remove inline comments from catalogs.go - Remove inline comments from format.go 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
fjakobs
force-pushed
the
mcp/pagination-support-list-tables
branch
from
801d346 to
01cc446
Compare
fjakobs
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
fjakobs
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
fjakobs
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Collaborator
|
Commit: 6453c5f
24 failing tests:
Top 50 slowest tests (at least 2 minutes):
|
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.
Summary