This is an automated email from the ASF dual-hosted git repository.
jerryshao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 0ed1108198 [#13122] fix(mcp): unify statistics metadata full-name
parameter (#13123)
0ed1108198 is described below
commit 0ed1108198ad1b5ba9ecdbe533dc5d5c4ef90f9a
Author: Qi Yu <[email protected]>
AuthorDate: Mon Sep 14 14:41:56 2026 +0800
[#13122] fix(mcp): unify statistics metadata full-name parameter (#13123)
### What changes were proposed in this pull request?
Rename the two statistics tools' public parameter to
`metadata_full_name`. Accept `metadata_fullname` as a deprecated input
alias using Pydantic validation aliases, while advertising only the
canonical name in the tool schema. Reject calls supplying both
spellings. Update documentation and regression tests.
### Why are the changes needed?
Statistics tools reject the `metadata_full_name` spelling used by tag
and policy tools, making callers fail validation when switching between
related tools.
Fix: #13122
### Does this PR introduce _any_ user-facing change?
Statistics tool schemas now use `metadata_full_name`, consistently with
tag and policy tools. Existing calls using `metadata_fullname` continue
to work. REST APIs are unchanged.
### How was this patch tested?
- Before the fix, both canonical-name calls and both statistics schema
checks fail.
- MCP unit suite: 271 tests and 24 subtests passed, covering the nine
related tool schemas, canonical and legacy calls, missing/duplicate
arguments, and per-request metalake selection.
- `./gradlew :mcp-server:spotlessApply`, targeted Pylint (10/10), and
`git diff --check` passed.
- Tests invoke MCP in process with mocked backend operations; no live
Gravitino server is required.
---
docs/gravitino-mcp-server.md | 2 +
mcp-server/mcp_server/tools/statistic.py | 31 +++++--
mcp-server/tests/unit/test_per_request_metalake.py | 2 +-
mcp-server/tests/unit/tools/test_statistic.py | 102 ++++++++++++++++++++-
4 files changed, 127 insertions(+), 10 deletions(-)
diff --git a/docs/gravitino-mcp-server.md b/docs/gravitino-mcp-server.md
index e97bdc6b69..d32770e4bb 100644
--- a/docs/gravitino-mcp-server.md
+++ b/docs/gravitino-mcp-server.md
@@ -254,6 +254,8 @@ Use the `list_metalakes` tool to discover which metalakes a
caller may use. It i
The statistic tools (`list_statistics_for_metadata`,
`list_statistics_for_partition`) shipped their own `metalake_name` argument
before metalake selection was unified. It is still accepted as a deprecated
alias for `metalake`, so existing callers keep working; passing both with
different values is rejected. New callers should use `metalake`.
+These tools use `metadata_full_name` for the metadata object name, consistent
with the tag and policy tools. The previous spelling, `metadata_fullname`, is
accepted as a deprecated input alias but is not advertised in the tool schema.
Supply only one spelling per call; passing both is rejected. New callers should
use `metadata_full_name`.
+
Authorization is unchanged — the caller's identity (see above) determines what
it may see in the named metalake exactly as it would through the REST API. Note
that a caller can now reach any metalake its credentials permit, so scope the
credentials accordingly when that matters.
### Examples
diff --git a/mcp-server/mcp_server/tools/statistic.py
b/mcp-server/mcp_server/tools/statistic.py
index 8e5365fcba..5d2f03ae13 100644
--- a/mcp-server/mcp_server/tools/statistic.py
+++ b/mcp-server/mcp_server/tools/statistic.py
@@ -15,7 +15,10 @@
# specific language governing permissions and limitations
# under the License.
+from typing import Annotated
+
from fastmcp import Context, FastMCP
+from pydantic import AliasChoices, Field
def load_statistic_tools(mcp: FastMCP):
@@ -23,12 +26,19 @@ def load_statistic_tools(mcp: FastMCP):
async def list_statistics_for_metadata(
ctx: Context,
metadata_type: str,
- metadata_fullname: str,
+ metadata_full_name: Annotated[
+ str,
+ Field(
+ validation_alias=AliasChoices(
+ "metadata_full_name", "metadata_fullname"
+ )
+ ),
+ ],
) -> str:
"""
Retrieve a list of statistics for a specific metadata object.
Currently,
this tool only supports statistics for tables, so `metadata_type`
- should always be "table" and metadata_fullname should be in the
format
+ should always be "table" and metadata_full_name should be in the
format
"{catalog}.{schema}.{table}". For more information about the
metadata
type and full name formats, please refer to the tool
'metadata_type_to_fullname_formats'.
@@ -37,7 +47,7 @@ def load_statistic_tools(mcp: FastMCP):
ctx (Context): The request context.
metadata_type (str): The type of metadata (e.g., table, column).
For
more, please refer to the tool
'metadata_type_to_fullname_formats'
- metadata_fullname (str): The full name of the metadata object. For
+ metadata_full_name (str): The full name of the metadata object. For
more, please refer to tool 'metadata_type_to_fullname_formats'.
@@ -68,7 +78,7 @@ def load_statistic_tools(mcp: FastMCP):
"""
client = ctx.request_context.lifespan_context.rest_client()
return await client.as_statistic_operation().list_of_statistics(
- metadata_type, metadata_fullname
+ metadata_type, metadata_full_name
)
# pylint: disable=R0917
@@ -76,7 +86,14 @@ def load_statistic_tools(mcp: FastMCP):
async def list_statistics_for_partition(
ctx: Context,
metadata_type: str,
- metadata_fullname: str,
+ metadata_full_name: Annotated[
+ str,
+ Field(
+ validation_alias=AliasChoices(
+ "metadata_full_name", "metadata_fullname"
+ )
+ ),
+ ],
from_partition_name: str,
to_partition_name: str,
from_inclusive: bool = True,
@@ -90,7 +107,7 @@ def load_statistic_tools(mcp: FastMCP):
Args:
ctx (Context): The request context.
metadata_type (str): The type of metadata, should be "table" for
partition statistics.
- metadata_fullname (str): The full name of the metadata item, the
format should be
+ metadata_full_name (str): The full name of the metadata item, the
format should be
"{catalog}.{schema}.{table}".
from_partition_name (str): Starting partition name.
to_partition_name (str): Ending partition name.
@@ -133,7 +150,7 @@ def load_statistic_tools(mcp: FastMCP):
return (
await client.as_statistic_operation().list_statistic_for_partition(
metadata_type,
- metadata_fullname,
+ metadata_full_name,
from_partition_name,
to_partition_name,
from_inclusive,
diff --git a/mcp-server/tests/unit/test_per_request_metalake.py
b/mcp-server/tests/unit/test_per_request_metalake.py
index 68ec723e83..95faea1abb 100644
--- a/mcp-server/tests/unit/test_per_request_metalake.py
+++ b/mcp-server/tests/unit/test_per_request_metalake.py
@@ -708,7 +708,7 @@ class TestDeprecatedMetalakeNameAlias(unittest.TestCase):
return asyncio.run(_run())
def _base(self):
- return {"metadata_type": "table", "metadata_fullname": "c.s.t"}
+ return {"metadata_type": "table", "metadata_full_name": "c.s.t"}
def test_legacy_argument_still_selects_the_metalake(self):
self._call({**self._base(), "metalake_name": "legacy_ml"})
diff --git a/mcp-server/tests/unit/tools/test_statistic.py
b/mcp-server/tests/unit/tools/test_statistic.py
index 1140cd393a..560b7eeef4 100644
--- a/mcp-server/tests/unit/tools/test_statistic.py
+++ b/mcp-server/tests/unit/tools/test_statistic.py
@@ -19,6 +19,7 @@ import asyncio
import unittest
from fastmcp import Client
+from fastmcp.exceptions import ToolError
from mcp_server.client.factory import RESTClientFactory
from mcp_server.core import Setting
@@ -39,7 +40,7 @@ class TestStatisticTool(unittest.TestCase):
"list_statistics_for_metadata",
{
"metadata_type": "mock_type",
- "metadata_fullname": "mock_fullname",
+ "metadata_full_name": "mock_fullname",
},
)
self.assertEqual(
@@ -56,7 +57,7 @@ class TestStatisticTool(unittest.TestCase):
"list_statistics_for_partition",
{
"metadata_type": "mock_type",
- "metadata_fullname": "mock_fullname",
+ "metadata_full_name": "mock_fullname",
"from_partition_name": "from_partition",
"to_partition_name": "to_partition",
},
@@ -68,3 +69,100 @@ class TestStatisticTool(unittest.TestCase):
)
asyncio.run(_test_list_statistics_for_partition(self.mcp))
+
+ def test_metadata_full_name_schema_is_consistent(self):
+ async def _test():
+ async with Client(self.mcp) as client:
+ tools = {tool.name: tool for tool in await client.list_tools()}
+ for name in (
+ "list_statistics_for_metadata",
+ "list_statistics_for_partition",
+ "associate_tag_with_metadata",
+ "disassociate_tag_from_metadata",
+ "list_tags_for_metadata",
+ "associate_policy_with_metadata",
+ "disassociate_policy_from_metadata",
+ "list_policies_for_metadata",
+ "get_policy_for_metadata",
+ ):
+ with self.subTest(tool=name):
+ schema = tools[name].inputSchema
+ self.assertIn(
+ "metadata_full_name", schema["properties"]
+ )
+ self.assertIn("metadata_full_name", schema["required"])
+ self.assertNotIn(
+ "metadata_fullname", schema["properties"]
+ )
+
+ asyncio.run(_test())
+
+ def test_legacy_metadata_fullname_is_still_accepted(self):
+ async def _test():
+ async with Client(self.mcp) as client:
+ for name, extra, expected in (
+ (
+ "list_statistics_for_metadata",
+ {},
+ "mock_statistics: table, catalog.schema.table",
+ ),
+ (
+ "list_statistics_for_partition",
+ {
+ "from_partition_name": "p1",
+ "to_partition_name": "p2",
+ "from_inclusive": False,
+ "to_inclusive": True,
+ },
+ "mock_statistics_for_partition: table, "
+ "catalog.schema.table, p1, p2, False, True",
+ ),
+ ):
+ with self.subTest(tool=name):
+ result = await client.call_tool(
+ name,
+ {
+ "metadata_type": "table",
+ "metadata_fullname": "catalog.schema.table",
+ **extra,
+ },
+ )
+ self.assertEqual(result.content[0].text, expected)
+
+ asyncio.run(_test())
+
+ def test_missing_or_duplicate_full_name_is_rejected(self):
+ async def _test():
+ async with Client(self.mcp) as client:
+ for name, extra in (
+ ("list_statistics_for_metadata", {}),
+ (
+ "list_statistics_for_partition",
+ {
+ "from_partition_name": "p1",
+ "to_partition_name": "p2",
+ },
+ ),
+ ):
+ for arguments, error in (
+ ({}, "Missing required argument"),
+ (
+ {
+ "metadata_full_name": "catalog.schema.table",
+ "metadata_fullname": "catalog.schema.other",
+ },
+ "Unexpected keyword argument",
+ ),
+ ):
+ with self.subTest(tool=name, arguments=arguments):
+ with self.assertRaisesRegex(ToolError, error):
+ await client.call_tool(
+ name,
+ {
+ "metadata_type": "table",
+ **extra,
+ **arguments,
+ },
+ )
+
+ asyncio.run(_test())
