aminghadersohi commented on code in PR #40358:
URL: https://github.com/apache/superset/pull/40358#discussion_r3305904999


##########
superset/mcp_service/tag/tool/create_tag.py:
##########
@@ -0,0 +1,111 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import logging
+
+from fastmcp import Context
+from superset_core.mcp.decorators import tool, ToolAnnotations
+
+from superset.extensions import event_logger
+from superset.mcp_service.tag.schemas import CreateTagRequest, 
CreateTagResponse
+
+logger = logging.getLogger(__name__)
+
+
+@tool(
+    tags=["mutate"],
+    class_permission_name="Tag",
+    method_permission_name="write",
+    annotations=ToolAnnotations(
+        title="Create a tag",
+        readOnlyHint=False,
+        destructiveHint=False,
+    ),
+)
+async def create_tag(request: CreateTagRequest, ctx: Context) -> 
CreateTagResponse:
+    """Create a new custom tag in Superset, optionally applying it to objects.
+
+    Use this tool to create tags for organizing charts, dashboards, datasets,
+    and queries. Tags can be applied to multiple objects at creation time.
+
+    Workflow:
+    1. Call this tool with a tag name and optional description
+    2. Optionally provide objects_to_tag to apply the tag immediately
+    3. Use the returned ``id`` to reference the tag in future operations
+    """
+    await ctx.info("Creating tag: name=%r" % (request.name,))
+
+    try:
+        from superset.commands.tag.create import 
CreateCustomTagWithRelationshipsCommand
+        from superset.commands.tag.exceptions import (
+            TagCreateFailedError,
+            TagInvalidError,
+        )
+        from superset.daos.tag import TagDAO
+
+        with event_logger.log_context(action="mcp.create_tag.create"):
+            properties = {
+                "name": request.name,
+                "description": request.description or "",
+                "objects_to_tag": request.objects_to_tag,
+            }
+            objects_tagged, objects_skipped = 
CreateCustomTagWithRelationshipsCommand(

Review Comment:
   Fixed: the response now returns `tag.description` (the persisted value from 
the DB) rather than `request.description`. This ensures the client always sees 
what was actually saved.



##########
superset/mcp_service/tag/schemas.py:
##########
@@ -0,0 +1,68 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Pydantic schemas for tag-related MCP tools."""
+
+from pydantic import BaseModel, ConfigDict, Field
+
+
+class CreateTagRequest(BaseModel):
+    model_config = ConfigDict(populate_by_name=True)
+
+    name: str = Field(
+        ...,
+        min_length=1,
+        description=(
+            "Name for the new tag. Must be unique. "
+            "Used to identify and reference the tag across Superset."
+        ),
+    )
+    description: str | None = Field(
+        None,
+        description="Optional human-readable description for the tag.",
+    )
+    objects_to_tag: list[tuple[str, int]] = Field(
+        default_factory=list,
+        description=(
+            "Optional list of objects to apply this tag to immediately after 
creation. "
+            "Each item is a [object_type, object_id] pair where object_type is 
one of: "
+            "'chart', 'dashboard', 'dataset', 'query'."
+        ),
+    )

Review Comment:
   Fixed: added `max_length=250` to the `name` field (matching `Tag.name = 
String(250)`), a `field_validator` that strips whitespace and rejects 
whitespace-only names, and a `field_validator` on `objects_to_tag` that 
enforces `object_id >= 1` (matching the REST API's `Range(min=1)`).



##########
superset/mcp_service/app.py:
##########
@@ -670,6 +670,9 @@ def create_mcp_app(
     get_schema,
     health_check,
 )
+from superset.mcp_service.tag.tool import (  # noqa: F401, E402
+    create_tag,
+)

Review Comment:
   Fixed: added a 'Tag Management' section to the Available tools list and 
included `create_tag` in the write tools permission-awareness bullet.



##########
superset/mcp_service/tag/tool/create_tag.py:
##########
@@ -0,0 +1,111 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import logging
+
+from fastmcp import Context
+from superset_core.mcp.decorators import tool, ToolAnnotations
+
+from superset.extensions import event_logger
+from superset.mcp_service.tag.schemas import CreateTagRequest, 
CreateTagResponse
+
+logger = logging.getLogger(__name__)
+
+
+@tool(
+    tags=["mutate"],
+    class_permission_name="Tag",
+    method_permission_name="write",
+    annotations=ToolAnnotations(
+        title="Create a tag",
+        readOnlyHint=False,
+        destructiveHint=False,
+    ),
+)
+async def create_tag(request: CreateTagRequest, ctx: Context) -> 
CreateTagResponse:
+    """Create a new custom tag in Superset, optionally applying it to objects.
+
+    Use this tool to create tags for organizing charts, dashboards, datasets,
+    and queries. Tags can be applied to multiple objects at creation time.
+
+    Workflow:
+    1. Call this tool with a tag name and optional description
+    2. Optionally provide objects_to_tag to apply the tag immediately
+    3. Use the returned ``id`` to reference the tag in future operations
+    """
+    await ctx.info("Creating tag: name=%r" % (request.name,))

Review Comment:
   Fixed: added `tests/unit_tests/mcp_service/tag/tool/test_create_tag.py` 
covering the happy path (with and without objects), skipped objects, whitespace 
stripping, `TagInvalidError`, `TagCreateFailedError`, and schema validation 
rejection of blank names and invalid object IDs.



##########
superset/mcp_service/tag/tool/create_tag.py:
##########
@@ -0,0 +1,111 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import logging
+
+from fastmcp import Context
+from superset_core.mcp.decorators import tool, ToolAnnotations
+
+from superset.extensions import event_logger
+from superset.mcp_service.tag.schemas import CreateTagRequest, 
CreateTagResponse
+
+logger = logging.getLogger(__name__)
+
+
+@tool(
+    tags=["mutate"],
+    class_permission_name="Tag",
+    method_permission_name="write",
+    annotations=ToolAnnotations(
+        title="Create a tag",
+        readOnlyHint=False,
+        destructiveHint=False,
+    ),
+)
+async def create_tag(request: CreateTagRequest, ctx: Context) -> 
CreateTagResponse:
+    """Create a new custom tag in Superset, optionally applying it to objects.
+
+    Use this tool to create tags for organizing charts, dashboards, datasets,
+    and queries. Tags can be applied to multiple objects at creation time.
+
+    Workflow:
+    1. Call this tool with a tag name and optional description
+    2. Optionally provide objects_to_tag to apply the tag immediately
+    3. Use the returned ``id`` to reference the tag in future operations
+    """
+    await ctx.info("Creating tag: name=%r" % (request.name,))
+
+    try:
+        from superset.commands.tag.create import 
CreateCustomTagWithRelationshipsCommand
+        from superset.commands.tag.exceptions import (
+            TagCreateFailedError,
+            TagInvalidError,
+        )
+        from superset.daos.tag import TagDAO
+
+        with event_logger.log_context(action="mcp.create_tag.create"):
+            properties = {
+                "name": request.name,
+                "description": request.description or "",
+                "objects_to_tag": request.objects_to_tag,
+            }
+            objects_tagged, objects_skipped = 
CreateCustomTagWithRelationshipsCommand(
+                properties
+            ).run()
+
+        tag = TagDAO.find_by_name(request.name)
+
+        await ctx.info(
+            "Tag created: id=%s, name=%r, objects_tagged=%d, 
objects_skipped=%d"
+            % (
+                tag.id if tag else None,
+                request.name,
+                len(objects_tagged),
+                len(objects_skipped),
+            )
+        )
+
+        return CreateTagResponse(
+            id=tag.id if tag else None,
+            name=request.name,
+            description=request.description,
+            objects_tagged=list(objects_tagged),
+            objects_skipped=list(objects_skipped),
+        )

Review Comment:
   Fixed: `sorted(objects_tagged)` and `sorted(objects_skipped)` are used 
before building the response, giving deterministic ordering by `(object_type, 
object_id)`.



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to