Copilot commented on code in PR #11621:
URL: https://github.com/apache/gravitino/pull/11621#discussion_r3542963764
##########
mcp-server/mcp_server/tools/tag.py:
##########
@@ -173,7 +174,8 @@ async def alter_tag(ctx: Context, name: str, updates: list)
-> str:
client = ctx.request_context.lifespan_context.rest_client()
return await client.as_tag_operation().alter_tag(name, updates)
- # Disable the delete_tag tool by default as it can be destructive.
+ # delete_tag is destructive; it is exposed and access is enforced by
Gravitino
+ # authorization rather than by hiding the tool.
@mcp.tool(tags={"tag"})
async def delete_tag(ctx: Context, name: str) -> None:
"""
Review Comment:
`delete_tag` is now exposed, but the tool (and underlying TagOperation)
returns `None` even though the Gravitino REST API documents tag deletion as
returning a DropResponse with a `dropped` boolean. This is inconsistent with
the other newly added drop/delete tools (schema/table/topic/fileset/model), and
it prevents MCP callers from programmatically confirming success.
Consider changing `delete_tag` to return the `dropped` value (as a JSON
boolean string, like other drop/delete tools) and updating the TagOperation
interface + plain/mock implementations/tests accordingly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
