This is an automated email from the ASF dual-hosted git repository. cgivre pushed a commit to branch feat/drill-mcp-server in repository https://gitbox.apache.org/repos/asf/drill-mcp.git
commit ee674e7adc6564843c7ca65593c30d25cbff5634 Author: cgivre <[email protected]> AuthorDate: Wed Aug 12 15:23:57 2026 -0400 fix: correct README config precedence and describe_table claims; tidy imports and typing README stated CLI > config file > environment; the code (and config.py's own docstring) has always applied CLI > environment > config file, later overriding earlier -- fixed the sentence and called out the credential implication explicitly. Dropped the pytest -m integration line, since no test currently carries that marker. Corrected the describe_table tool-table entry and column-discovery section: DESCRIBE reports nullability, the probe path does not and never has claimed to sanitise error text (that claim was never true and the code already propagates Drill's error unchanged on both paths). Moved tests/test_server.py's mid-file imports to the top block. build_client is now typed RestClient | JdbcClient via a TYPE_CHECKING-only import, keeping the lazy runtime import (and the JVM-free default install) unchanged. --- README.md | 23 ++++++++++++++++------- drill_mcp/server.py | 10 ++++++++-- tests/test_server.py | 10 +++------- 3 files changed, 27 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 777f153..faeeee3 100644 --- a/README.md +++ b/README.md @@ -32,11 +32,14 @@ Register it with an MCP client: ``` `drill-mcp --help` lists every flag; `drill.example.yaml` documents every -config key. CLI flags override the config file, which overrides the -`DRILL_*` environment variables, which override the built-in defaults. +config key. CLI flags override the `DRILL_*` environment variables, which +override the config file, which overrides the built-in defaults. Credentials (`user`/`password`) are read from the config file or from `DRILL_USER`/`DRILL_PASSWORD` only -- there is no `--user`/`--password` flag, -and no tool accepts a credential as an argument. +and no tool accepts a credential as an argument. Because the environment +wins over the config file, a stale exported `DRILL_PASSWORD` will silently +override a `password:` set in the config file -- `unset` it if you intend +the file to be authoritative. ## Tools @@ -45,7 +48,7 @@ and no tool accepts a credential as an argument. | `run_query` | Run one SQL statement | | `list_schemas` | List visible schemas | | `list_tables` | List tables in a schema | -| `describe_table` | Column names and types | +| `describe_table` | Column names, types, and nullability where the plugin reports it | | `list_storage_plugins` | Plugin configs, secrets redacted | | `cluster_status` | Drillbit membership and status | | `list_profiles` | Recent and running queries | @@ -116,12 +119,19 @@ registers its schema: - For plugins with a schema registered ahead of time (most JDBC-style and relational-style plugins), `describe_table` uses Drill's `DESCRIBE`, which - is metadata-only and never reads user data. + is metadata-only and never reads user data, and reports nullability for + each column. - For plugins whose schema is only known at read time (`file`, `mongo`, `splunk`), `DESCRIBE` cannot answer, so `describe_table` probes with `SELECT ... LIMIT 1` against the target instead. It returns **only column names and types** -- the sampled row itself is never included in the - result. + result, and never has been: this remains true regardless of how any given + probe query fails. Nullability is not derivable from a single sampled row, + so it is reported as `None` on this path rather than guessed. A probe + failure (missing table, permissions, a genuine data error) surfaces + Drill's error text unchanged, exactly like the `DESCRIBE` path -- Drill's + errors do not embed cell content, so there is nothing for this path to + suppress. - **HTTP plugins cannot report columns until a query has been run** against the endpoint; Drill has no schema for an HTTP source ahead of a real request. `describe_table` says this explicitly in its error rather than @@ -156,5 +166,4 @@ See `drill.example.yaml` for the full configuration reference. ```bash pip install -e ".[dev]" pytest # unit tests, no cluster or JVM needed -pytest -m integration # requires a live Drill at DRILL_URL ``` diff --git a/drill_mcp/server.py b/drill_mcp/server.py index 2db5747..2d9cae7 100644 --- a/drill_mcp/server.py +++ b/drill_mcp/server.py @@ -32,7 +32,7 @@ from __future__ import annotations import argparse import logging import sys -from typing import Any +from typing import TYPE_CHECKING, Any from mcp.server.mcpserver import MCPServer @@ -40,6 +40,12 @@ from .client_rest import DrillError, RestClient from .config import Config, ConfigError, load_config from .guard import Policy, PolicyError, check, is_show_command, matches_prefix +if TYPE_CHECKING: + # Imported only for the type checker: build_client's lazy, in-function + # import of JdbcClient (see below) is what keeps jaydebeapi/JPype1 out + # of the import graph for the REST-only, JVM-free default install. + from .client_jdbc import JdbcClient + class ToolError(Exception): """The single error type surfaced to MCP clients. Never carries a traceback.""" @@ -217,7 +223,7 @@ class DrillTools: raise ToolError(str(exc)) from exc -def build_client(config: Config) -> Any: +def build_client(config: Config) -> RestClient | JdbcClient: """Construct the wire client the configured backend calls for.""" if config.backend == "jdbc": from .client_jdbc import JdbcClient diff --git a/tests/test_server.py b/tests/test_server.py index 55fcf16..3678132 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -21,9 +21,10 @@ from unittest.mock import MagicMock import pytest -from drill_mcp.client_rest import DrillError, QueryResult +from drill_mcp.client_jdbc import JdbcClient +from drill_mcp.client_rest import DrillError, QueryResult, RestClient from drill_mcp.config import load_config -from drill_mcp.server import DrillTools, ToolError +from drill_mcp.server import DrillTools, ToolError, build_client, build_server def make_tools(client=None, **overrides): @@ -537,11 +538,6 @@ class TestShowFiltering: assert result["rows"] == [{"SCHEMA_NAME": "dfs.tmp"}] -from drill_mcp.client_jdbc import JdbcClient -from drill_mcp.client_rest import RestClient -from drill_mcp.server import build_client, build_server - - class TestWiring: def test_rest_backend_builds_a_rest_client(self): assert isinstance(build_client(load_config()), RestClient)
