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


##########
tests/unit_tests/mcp_service/semantic_layer/tool/test_get_table.py:
##########
@@ -297,3 +297,227 @@ async def 
test_get_table_external_time_range_without_dttm_validation_error(
     assert data["success"] is False
     assert data["error_type"] == "ValidationError"
     assert "no datetime dimension" in data["message"]
+
+
[email protected]
+async def test_get_table_dataset_not_found(mcp_server: FastMCP) -> None:
+    """get_table returns NotFound when dataset_id doesn't resolve to a 
dataset."""
+    with patch("superset.daos.dataset.DatasetDAO.find_by_id", 
return_value=None):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "get_table",
+                {"request": {"dataset_id": 999999, "metrics": ["revenue"]}},
+            )
+        data = json.loads(result.content[0].text)
+
+    assert data["success"] is False
+    assert data["error_type"] == "NotFound"
+    assert "999999" in data["message"]
+
+
[email protected]
+async def test_get_table_view_not_found(mcp_server: FastMCP) -> None:
+    """get_table returns NotFound when view_id doesn't resolve to a view."""
+    with patch(
+        "superset.daos.semantic_layer.SemanticViewDAO.find_by_id",
+        return_value=None,
+    ):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "get_table",
+                {"request": {"view_id": 999999, "metrics": ["bookings"]}},
+            )
+        data = json.loads(result.content[0].text)
+
+    assert data["success"] is False
+    assert data["error_type"] == "NotFound"
+    assert "999999" in data["message"]
+
+
[email protected]
+async def test_get_table_invalid_filter_column_validation_error(
+    mcp_server: FastMCP,
+) -> None:
+    """get_table errors when a filter references an unknown column."""
+    mock_ds = _make_dataset(42)
+
+    with patch("superset.daos.dataset.DatasetDAO.find_by_id", 
return_value=mock_ds):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "get_table",
+                {
+                    "request": {
+                        "dataset_id": 42,
+                        "metrics": ["revenue"],
+                        "filters": [{"col": "bogus_col", "op": "==", "val": 
"x"}],
+                    }
+                },
+            )
+        data = json.loads(result.content[0].text)
+
+    assert data["success"] is False
+    assert data["error_type"] == "ValidationError"
+    assert "Unknown filter column: 'bogus_col'" in data["error"]
+
+
[email protected]
+async def test_get_table_invalid_order_by_validation_error(
+    mcp_server: FastMCP,
+) -> None:
+    """get_table errors when order_by references an unknown column/metric."""
+    mock_ds = _make_dataset(42)
+
+    with patch("superset.daos.dataset.DatasetDAO.find_by_id", 
return_value=mock_ds):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "get_table",
+                {
+                    "request": {
+                        "dataset_id": 42,
+                        "metrics": ["revenue"],
+                        "order_by": ["bogus_order_col"],
+                    }
+                },
+            )
+        data = json.loads(result.content[0].text)
+
+    assert data["success"] is False
+    assert data["error_type"] == "ValidationError"
+    assert "Unknown order_by: 'bogus_order_col'" in data["error"]
+
+
[email protected]
+async def test_get_table_unknown_filter_operator_passes_through(
+    mcp_server: FastMCP,
+) -> None:
+    """An operator string outside the documented set is not schema-validated.
+
+    ``GetTableFilter.op`` is a plain ``str`` field (not a Literal/Enum), so
+    the tool does not reject unrecognized operator values itself -- it
+    forwards them verbatim to the query layer, which is responsible for
+    interpreting/rejecting them.
+    """
+    mock_ds = _make_dataset(42)
+    query_result = {

Review Comment:
   Added the requested explicit local type annotation in 8c643dd92d.



##########
tests/unit_tests/mcp_service/semantic_layer/tool/test_get_table.py:
##########
@@ -297,3 +297,227 @@ async def 
test_get_table_external_time_range_without_dttm_validation_error(
     assert data["success"] is False
     assert data["error_type"] == "ValidationError"
     assert "no datetime dimension" in data["message"]
+
+
[email protected]
+async def test_get_table_dataset_not_found(mcp_server: FastMCP) -> None:
+    """get_table returns NotFound when dataset_id doesn't resolve to a 
dataset."""
+    with patch("superset.daos.dataset.DatasetDAO.find_by_id", 
return_value=None):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "get_table",
+                {"request": {"dataset_id": 999999, "metrics": ["revenue"]}},
+            )
+        data = json.loads(result.content[0].text)
+
+    assert data["success"] is False
+    assert data["error_type"] == "NotFound"
+    assert "999999" in data["message"]
+
+
[email protected]
+async def test_get_table_view_not_found(mcp_server: FastMCP) -> None:
+    """get_table returns NotFound when view_id doesn't resolve to a view."""
+    with patch(
+        "superset.daos.semantic_layer.SemanticViewDAO.find_by_id",
+        return_value=None,
+    ):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "get_table",
+                {"request": {"view_id": 999999, "metrics": ["bookings"]}},
+            )
+        data = json.loads(result.content[0].text)
+
+    assert data["success"] is False
+    assert data["error_type"] == "NotFound"
+    assert "999999" in data["message"]
+
+
[email protected]
+async def test_get_table_invalid_filter_column_validation_error(
+    mcp_server: FastMCP,
+) -> None:
+    """get_table errors when a filter references an unknown column."""
+    mock_ds = _make_dataset(42)
+
+    with patch("superset.daos.dataset.DatasetDAO.find_by_id", 
return_value=mock_ds):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "get_table",
+                {
+                    "request": {
+                        "dataset_id": 42,
+                        "metrics": ["revenue"],
+                        "filters": [{"col": "bogus_col", "op": "==", "val": 
"x"}],
+                    }
+                },
+            )
+        data = json.loads(result.content[0].text)
+
+    assert data["success"] is False
+    assert data["error_type"] == "ValidationError"
+    assert "Unknown filter column: 'bogus_col'" in data["error"]
+
+
[email protected]
+async def test_get_table_invalid_order_by_validation_error(
+    mcp_server: FastMCP,
+) -> None:
+    """get_table errors when order_by references an unknown column/metric."""
+    mock_ds = _make_dataset(42)
+
+    with patch("superset.daos.dataset.DatasetDAO.find_by_id", 
return_value=mock_ds):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "get_table",
+                {
+                    "request": {
+                        "dataset_id": 42,
+                        "metrics": ["revenue"],
+                        "order_by": ["bogus_order_col"],
+                    }
+                },
+            )
+        data = json.loads(result.content[0].text)
+
+    assert data["success"] is False
+    assert data["error_type"] == "ValidationError"
+    assert "Unknown order_by: 'bogus_order_col'" in data["error"]
+
+
[email protected]
+async def test_get_table_unknown_filter_operator_passes_through(
+    mcp_server: FastMCP,
+) -> None:
+    """An operator string outside the documented set is not schema-validated.
+
+    ``GetTableFilter.op`` is a plain ``str`` field (not a Literal/Enum), so
+    the tool does not reject unrecognized operator values itself -- it
+    forwards them verbatim to the query layer, which is responsible for
+    interpreting/rejecting them.
+    """
+    mock_ds = _make_dataset(42)
+    query_result = {
+        "queries": [
+            {
+                "data": [{"region": "west", "revenue": 100}],
+                "colnames": ["region", "revenue"],
+                "rowcount": 1,
+            }
+        ]
+    }
+
+    with (
+        patch("superset.daos.dataset.DatasetDAO.find_by_id", 
return_value=mock_ds),
+        patch(
+            "superset.commands.chart.data.get_data_command.ChartDataCommand"
+        ) as mock_command_cls,
+        patch(
+            "superset.common.query_context_factory.QueryContextFactory"
+        ) as mock_factory_cls,
+    ):
+        mock_command_cls.return_value.run.return_value = query_result
+        mock_factory_cls.return_value.create.return_value = MagicMock()
+
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "get_table",
+                {
+                    "request": {
+                        "dataset_id": 42,
+                        "metrics": ["revenue"],
+                        "filters": [
+                            {"col": "region", "op": "TOTALLY_BOGUS_OP", "val": 
"x"}
+                        ],
+                    }
+                },
+            )
+        data = json.loads(result.content[0].text)
+
+        create_kwargs = mock_factory_cls.return_value.create.call_args.kwargs
+        forwarded_filters = create_kwargs["queries"][0]["filters"]
+
+    assert data["success"] is True
+    assert {"col": "region", "op": "TOTALLY_BOGUS_OP", "val": "x"} in 
forwarded_filters
+
+
[email protected]
+async def test_get_table_unicode_filter_value_passes_through(
+    mcp_server: FastMCP,
+) -> None:
+    """Unicode filter values are forwarded to the query layer unmodified."""
+    mock_ds = _make_dataset(42)
+    query_result = {

Review Comment:
   Added the requested explicit local type annotation in 8c643dd92d.



##########
tests/unit_tests/mcp_service/semantic_layer/tool/test_get_table.py:
##########
@@ -297,3 +297,227 @@ async def 
test_get_table_external_time_range_without_dttm_validation_error(
     assert data["success"] is False
     assert data["error_type"] == "ValidationError"
     assert "no datetime dimension" in data["message"]
+
+
[email protected]
+async def test_get_table_dataset_not_found(mcp_server: FastMCP) -> None:
+    """get_table returns NotFound when dataset_id doesn't resolve to a 
dataset."""
+    with patch("superset.daos.dataset.DatasetDAO.find_by_id", 
return_value=None):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "get_table",
+                {"request": {"dataset_id": 999999, "metrics": ["revenue"]}},
+            )
+        data = json.loads(result.content[0].text)
+
+    assert data["success"] is False
+    assert data["error_type"] == "NotFound"
+    assert "999999" in data["message"]
+
+
[email protected]
+async def test_get_table_view_not_found(mcp_server: FastMCP) -> None:
+    """get_table returns NotFound when view_id doesn't resolve to a view."""
+    with patch(
+        "superset.daos.semantic_layer.SemanticViewDAO.find_by_id",
+        return_value=None,
+    ):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "get_table",
+                {"request": {"view_id": 999999, "metrics": ["bookings"]}},
+            )
+        data = json.loads(result.content[0].text)
+
+    assert data["success"] is False
+    assert data["error_type"] == "NotFound"
+    assert "999999" in data["message"]
+
+
[email protected]
+async def test_get_table_invalid_filter_column_validation_error(
+    mcp_server: FastMCP,
+) -> None:
+    """get_table errors when a filter references an unknown column."""
+    mock_ds = _make_dataset(42)
+
+    with patch("superset.daos.dataset.DatasetDAO.find_by_id", 
return_value=mock_ds):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "get_table",
+                {
+                    "request": {
+                        "dataset_id": 42,
+                        "metrics": ["revenue"],
+                        "filters": [{"col": "bogus_col", "op": "==", "val": 
"x"}],
+                    }
+                },
+            )
+        data = json.loads(result.content[0].text)
+
+    assert data["success"] is False
+    assert data["error_type"] == "ValidationError"
+    assert "Unknown filter column: 'bogus_col'" in data["error"]
+
+
[email protected]
+async def test_get_table_invalid_order_by_validation_error(
+    mcp_server: FastMCP,
+) -> None:
+    """get_table errors when order_by references an unknown column/metric."""
+    mock_ds = _make_dataset(42)
+
+    with patch("superset.daos.dataset.DatasetDAO.find_by_id", 
return_value=mock_ds):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "get_table",
+                {
+                    "request": {
+                        "dataset_id": 42,
+                        "metrics": ["revenue"],
+                        "order_by": ["bogus_order_col"],
+                    }
+                },
+            )
+        data = json.loads(result.content[0].text)
+
+    assert data["success"] is False
+    assert data["error_type"] == "ValidationError"
+    assert "Unknown order_by: 'bogus_order_col'" in data["error"]
+
+
[email protected]
+async def test_get_table_unknown_filter_operator_passes_through(
+    mcp_server: FastMCP,
+) -> None:
+    """An operator string outside the documented set is not schema-validated.
+
+    ``GetTableFilter.op`` is a plain ``str`` field (not a Literal/Enum), so
+    the tool does not reject unrecognized operator values itself -- it
+    forwards them verbatim to the query layer, which is responsible for
+    interpreting/rejecting them.
+    """
+    mock_ds = _make_dataset(42)
+    query_result = {
+        "queries": [
+            {
+                "data": [{"region": "west", "revenue": 100}],
+                "colnames": ["region", "revenue"],
+                "rowcount": 1,
+            }
+        ]
+    }
+
+    with (
+        patch("superset.daos.dataset.DatasetDAO.find_by_id", 
return_value=mock_ds),
+        patch(
+            "superset.commands.chart.data.get_data_command.ChartDataCommand"
+        ) as mock_command_cls,
+        patch(
+            "superset.common.query_context_factory.QueryContextFactory"
+        ) as mock_factory_cls,
+    ):
+        mock_command_cls.return_value.run.return_value = query_result
+        mock_factory_cls.return_value.create.return_value = MagicMock()
+
+        async with Client(mcp_server) as client:
+            result = await client.call_tool(
+                "get_table",
+                {
+                    "request": {
+                        "dataset_id": 42,
+                        "metrics": ["revenue"],
+                        "filters": [
+                            {"col": "region", "op": "TOTALLY_BOGUS_OP", "val": 
"x"}
+                        ],
+                    }
+                },
+            )
+        data = json.loads(result.content[0].text)
+
+        create_kwargs = mock_factory_cls.return_value.create.call_args.kwargs
+        forwarded_filters = create_kwargs["queries"][0]["filters"]
+
+    assert data["success"] is True
+    assert {"col": "region", "op": "TOTALLY_BOGUS_OP", "val": "x"} in 
forwarded_filters
+
+
[email protected]
+async def test_get_table_unicode_filter_value_passes_through(
+    mcp_server: FastMCP,
+) -> None:
+    """Unicode filter values are forwarded to the query layer unmodified."""
+    mock_ds = _make_dataset(42)
+    query_result = {
+        "queries": [
+            {
+                "data": [{"region": "west", "revenue": 100}],
+                "colnames": ["region", "revenue"],
+                "rowcount": 1,
+            }
+        ]
+    }
+    unicode_val = "日本語 café €"

Review Comment:
   Added the requested explicit local type annotation in 8c643dd92d.



##########
tests/unit_tests/mcp_service/system/tool/test_health_check.py:
##########
@@ -17,7 +17,38 @@
 
 """Tests for health_check MCP tool."""
 
+import importlib
+from unittest.mock import Mock, patch
+
+import pytest
+from fastmcp import Client
+
+from superset.mcp_service.app import mcp
 from superset.mcp_service.system.schemas import HealthCheckResponse
+from superset.utils import json
+
+# Import the submodule directly so ``patch.object`` targets the module (not the
+# ``health_check`` function that ``tool/__init__.py`` re-exports onto the
+# package).
+health_check_module = importlib.import_module(
+    "superset.mcp_service.system.tool.health_check"
+)
+
+
[email protected]
+def mcp_server():
+    return mcp

Review Comment:
   Added explicit parameter/return annotations for the added test or fixture in 
8c643dd92d.



##########
tests/unit_tests/mcp_service/system/tool/test_health_check.py:
##########
@@ -17,7 +17,38 @@
 
 """Tests for health_check MCP tool."""
 
+import importlib
+from unittest.mock import Mock, patch
+
+import pytest
+from fastmcp import Client
+
+from superset.mcp_service.app import mcp
 from superset.mcp_service.system.schemas import HealthCheckResponse
+from superset.utils import json
+
+# Import the submodule directly so ``patch.object`` targets the module (not the
+# ``health_check`` function that ``tool/__init__.py`` re-exports onto the
+# package).
+health_check_module = importlib.import_module(
+    "superset.mcp_service.system.tool.health_check"
+)
+
+
[email protected]
+def mcp_server():
+    return mcp
+
+
[email protected](autouse=True)
+def mock_auth():
+    """Mock authentication for all tests."""
+    with patch("superset.mcp_service.auth.get_user_from_request") as 
mock_get_user:
+        mock_user = Mock()
+        mock_user.id = 1
+        mock_user.username = "admin"
+        mock_get_user.return_value = mock_user
+        yield mock_get_user

Review Comment:
   Added explicit parameter/return annotations for the added test or fixture in 
8c643dd92d.



##########
tests/unit_tests/mcp_service/system/tool/test_health_check.py:
##########
@@ -53,3 +84,94 @@ def test_health_check_response_with_uptime():
     )
 
     assert response.uptime_seconds == 123.45
+
+
+# ---------------------------------------------------------------------------
+# Tool-level tests: health_check via MCP Client
+# ---------------------------------------------------------------------------
+
+
[email protected]
+async def test_health_check_success_via_client(mcp_server):

Review Comment:
   Added explicit parameter/return annotations for the added test or fixture in 
8c643dd92d.



##########
tests/unit_tests/mcp_service/system/tool/test_health_check.py:
##########
@@ -53,3 +84,94 @@ def test_health_check_response_with_uptime():
     )
 
     assert response.uptime_seconds == 123.45
+
+
+# ---------------------------------------------------------------------------
+# Tool-level tests: health_check via MCP Client
+# ---------------------------------------------------------------------------
+
+
[email protected]
+async def test_health_check_success_via_client(mcp_server):
+    """Happy path: tool returns a healthy status with real system info."""
+    with patch.object(
+        health_check_module,
+        "get_version_metadata",
+        return_value={"version_string": "4.1.0"},
+    ):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool("health_check", {})
+
+    data = json.loads(result.content[0].text)
+    assert data["status"] == "healthy"
+    assert data["service"] == "Superset MCP Service"
+    assert data["version"] == "4.1.0"
+    assert data["timestamp"] is not None
+    assert data["uptime_seconds"] is not None
+    assert data["uptime_seconds"] >= 0
+
+
[email protected]
+async def test_health_check_uses_configured_app_name(mcp_server, app):

Review Comment:
   Added explicit parameter/return annotations for the added test or fixture in 
8c643dd92d.



##########
tests/unit_tests/mcp_service/system/tool/test_health_check.py:
##########
@@ -53,3 +84,94 @@ def test_health_check_response_with_uptime():
     )
 
     assert response.uptime_seconds == 123.45
+
+
+# ---------------------------------------------------------------------------
+# Tool-level tests: health_check via MCP Client
+# ---------------------------------------------------------------------------
+
+
[email protected]
+async def test_health_check_success_via_client(mcp_server):
+    """Happy path: tool returns a healthy status with real system info."""
+    with patch.object(
+        health_check_module,
+        "get_version_metadata",
+        return_value={"version_string": "4.1.0"},
+    ):
+        async with Client(mcp_server) as client:
+            result = await client.call_tool("health_check", {})
+
+    data = json.loads(result.content[0].text)
+    assert data["status"] == "healthy"
+    assert data["service"] == "Superset MCP Service"
+    assert data["version"] == "4.1.0"
+    assert data["timestamp"] is not None
+    assert data["uptime_seconds"] is not None
+    assert data["uptime_seconds"] >= 0
+
+
[email protected]
+async def test_health_check_uses_configured_app_name(mcp_server, app):
+    """service name is derived from the APP_NAME config, not hardcoded."""
+    app.config["APP_NAME"] = "Acme Analytics"
+    try:
+        with patch.object(
+            health_check_module,
+            "get_version_metadata",
+            return_value={"version_string": "4.1.0"},
+        ):
+            async with Client(mcp_server) as client:
+                result = await client.call_tool("health_check", {})
+    finally:
+        app.config.pop("APP_NAME", None)
+
+    data = json.loads(result.content[0].text)
+    assert data["service"] == "Acme Analytics MCP Service"
+
+
[email protected]
+async def test_health_check_returns_error_status_when_version_metadata_raises(
+    mcp_server,
+):

Review Comment:
   Added explicit parameter/return annotations for the added test or fixture in 
8c643dd92d.



##########
tests/unit_tests/mcp_service/test_rbac_tool_enforcement.py:
##########
@@ -0,0 +1,256 @@
+# 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.
+
+"""
+End-to-end RBAC-rejection coverage for mutating MCP tools.
+
+``tests/unit_tests/mcp_service/conftest.py`` disables RBAC for every other
+test module in this package (``disable_mcp_rbac``, autouse), and
+``test_auth_rbac.py`` unit-tests ``check_tool_permission`` in isolation by
+calling it directly with hand-built stub functions. Neither proves that a
+*real, registered* MCP tool actually enforces RBAC when invoked through the
+full FastMCP tool-call path (``Client(mcp).call_tool(...)`` ->
+``mcp_auth_hook`` -> ``check_tool_permission`` -> the tool body).
+
+This module closes that gap: for each mutating tool below, it re-enables
+RBAC, denies the mocked ``security_manager.can_access`` check, calls the
+*actual registered tool* through ``fastmcp.Client``, and asserts the call is
+rejected with a ``fastmcp.exceptions.ToolError`` before the tool body runs.
+It also includes one control test proving a permitted caller is NOT blocked
+by the RBAC gate, so the denial tests above cannot be a false negative caused
+by the test harness itself (e.g. a client/transport error that looks like a
+rejection for unrelated reasons).
+
+Scope: only the RBAC-rejection path. General tool behavior (happy path,
+validation errors, DAO error handling, etc.) is already covered by each
+tool's own test module under ``tests/unit_tests/mcp_service/<module>/tool/``
+and is intentionally not duplicated here.
+"""
+
+from collections.abc import Iterator
+from typing import Any
+from unittest.mock import MagicMock, patch
+
+import pytest
+from fastmcp import Client
+from fastmcp.exceptions import ToolError
+
+from superset.mcp_service.app import mcp
+
+# (tool_name, minimal-but-valid request payload, method_permission_name,
+# class_permission_name) for each mutating tool audited for RBAC enforcement.
+# The request payloads are the smallest bodies that pass each tool's Pydantic
+# schema validation (which FastMCP performs while binding arguments, before
+# ``mcp_auth_hook`` runs) so the call reaches the RBAC gate itself.
+_MUTATING_TOOLS: list[tuple[str, dict[str, Any], str, str]] = [
+    (
+        "execute_sql",
+        {"database_id": 1, "sql": "SELECT 1"},
+        "execute_sql_query",
+        "SQLLab",
+    ),
+    (
+        "update_chart",
+        {"identifier": 1},
+        "write",
+        "Chart",
+    ),
+    (
+        "update_dashboard",
+        {"identifier": 1},
+        "write",
+        "Dashboard",
+    ),
+    (
+        "generate_chart",
+        {
+            "dataset_id": 1,
+            "config": {"chart_type": "table", "columns": [{"name": "col1"}]},
+        },
+        "write",
+        "Chart",
+    ),
+    (
+        "generate_dashboard",
+        {"chart_ids": [1]},
+        "write",
+        "Dashboard",
+    ),
+    (
+        "manage_native_filters",
+        # ``reorder: []`` satisfies ManageNativeFiltersRequest's "at least one
+        # operation" validator (checked via ``is None``, not falsiness).
+        {"dashboard_id": 1, "reorder": []},
+        "write",
+        "Dashboard",
+    ),
+    (
+        "create_dataset",
+        {"database_id": 1, "table_name": "my_table"},
+        "write",
+        "Dataset",
+    ),
+    (
+        "create_virtual_dataset",
+        {"database_id": 1, "sql": "SELECT 1", "dataset_name": 
"my_virtual_dataset"},
+        "write",
+        "Dataset",
+    ),
+    (
+        "create_theme",
+        {"theme_name": "Denied Theme", "json_data": {"token": {}}},
+        "write",
+        "Theme",
+    ),
+    (
+        "save_sql_query",
+        {"database_id": 1, "label": "my query", "sql": "SELECT 1"},
+        "write",
+        "SavedQuery",
+    ),
+]
+
+_TOOL_IDS = [name for name, *_ in _MUTATING_TOOLS]

Review Comment:
   Added the explicit `list[str]` annotation to `_TOOL_IDS` in 8c643dd92d.



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