This is an automated email from the ASF dual-hosted git repository. kasiazjc pushed a commit to branch fix/column-select-saved-tab-label in repository https://gitbox.apache.org/repos/asf/superset.git
commit d56c85205128bf5a64e46175709a68677aad25e2 Author: Amin Ghadersohi <[email protected]> AuthorDate: Thu May 14 23:55:50 2026 +0000 fix(mcp): fix create_dataset exception classification and test correctness - CreateDatasetCommand.validate() wraps DatasetExistsValidationError and TableNotFoundValidationError inside DatasetInvalidError; the individual except branches were dead code. Now inspect exc._exceptions to classify. - Fix test patch target from tool module (lazy import) to source module superset.commands.dataset.create.CreateDatasetCommand. - Fix exception mock shape in exists/not-found tests to match real command behavior: DatasetInvalidError wrapping the specific validation error. --- .../mcp_service/dataset/tool/create_dataset.py | 14 +++--- .../dataset/tool/test_create_dataset.py | 51 +++++++++++++++------- 2 files changed, 43 insertions(+), 22 deletions(-) diff --git a/superset/mcp_service/dataset/tool/create_dataset.py b/superset/mcp_service/dataset/tool/create_dataset.py index ad37c24e592..5bb660ca8c4 100644 --- a/superset/mcp_service/dataset/tool/create_dataset.py +++ b/superset/mcp_service/dataset/tool/create_dataset.py @@ -97,13 +97,15 @@ async def create_dataset( ) return result - except DatasetExistsValidationError as exc: - await ctx.warning("Dataset already exists: %s" % str(exc)) - return DatasetError.create(error=str(exc), error_type="DatasetExistsError") - except TableNotFoundValidationError as exc: - await ctx.warning("Table not found: %s" % str(exc)) - return DatasetError.create(error=str(exc), error_type="TableNotFoundError") except DatasetInvalidError as exc: + # CreateDatasetCommand.validate() aggregates individual validation errors + # into DatasetInvalidError; inspect them for specific error types. + if any(isinstance(e, DatasetExistsValidationError) for e in exc._exceptions): + await ctx.warning("Dataset already exists: %s" % str(exc)) + return DatasetError.create(error=str(exc), error_type="DatasetExistsError") + if any(isinstance(e, TableNotFoundValidationError) for e in exc._exceptions): + await ctx.warning("Table not found: %s" % str(exc)) + return DatasetError.create(error=str(exc), error_type="TableNotFoundError") messages = exc.normalized_messages() await ctx.warning("Dataset validation failed: %s" % (messages,)) return DatasetError.create(error=str(messages), error_type="ValidationError") diff --git a/tests/unit_tests/mcp_service/dataset/tool/test_create_dataset.py b/tests/unit_tests/mcp_service/dataset/tool/test_create_dataset.py index b65a3296839..21e3bec329f 100644 --- a/tests/unit_tests/mcp_service/dataset/tool/test_create_dataset.py +++ b/tests/unit_tests/mcp_service/dataset/tool/test_create_dataset.py @@ -30,6 +30,9 @@ from superset.utils import json logging.basicConfig(level=logging.DEBUG) logger = logging.getLogger(__name__) +# Patch at source so lazy imports inside the tool function are intercepted. +_CMD_PATH = "superset.commands.dataset.create.CreateDatasetCommand" + def _make_mock_dataset( dataset_id: int = 42, @@ -88,7 +91,7 @@ def mock_auth(): class TestCreateDataset: """Tests for the create_dataset MCP tool.""" - @patch("superset.mcp_service.dataset.tool.create_dataset.CreateDatasetCommand") + @patch(_CMD_PATH) @pytest.mark.asyncio async def test_create_dataset_success(self, mock_command_class, mcp_server): """Happy path: tool creates dataset and returns DatasetInfo.""" @@ -121,7 +124,7 @@ class TestCreateDataset: assert call_kwargs["table_name"] == "orders" assert "owners" not in call_kwargs - @patch("superset.mcp_service.dataset.tool.create_dataset.CreateDatasetCommand") + @patch(_CMD_PATH) @pytest.mark.asyncio async def test_create_dataset_with_owners(self, mock_command_class, mcp_server): """Owners list is forwarded to the command when supplied.""" @@ -149,17 +152,25 @@ class TestCreateDataset: call_kwargs = mock_command_class.call_args[0][0] assert call_kwargs["owners"] == [5, 10] - @patch("superset.mcp_service.dataset.tool.create_dataset.CreateDatasetCommand") + @patch(_CMD_PATH) @pytest.mark.asyncio async def test_create_dataset_already_exists(self, mock_command_class, mcp_server): - """Returns DatasetError when a dataset for the table already exists.""" - from superset.commands.dataset.exceptions import DatasetExistsValidationError + """Returns DatasetExistsError when a dataset for the table already exists. + + CreateDatasetCommand.validate() wraps DatasetExistsValidationError inside + DatasetInvalidError, so simulate the real command shape. + """ + from superset.commands.dataset.exceptions import ( + DatasetExistsValidationError, + DatasetInvalidError, + ) from superset.sql.parse import Table + exc = DatasetInvalidError() + exc.append(DatasetExistsValidationError(Table("orders", "public", None))) + mock_command = MagicMock() - mock_command.run.side_effect = DatasetExistsValidationError( - Table("orders", "public", None) - ) + mock_command.run.side_effect = exc mock_command_class.return_value = mock_command async with Client(mcp_server) as client: @@ -178,17 +189,25 @@ class TestCreateDataset: assert data["error_type"] == "DatasetExistsError" assert "error" in data - @patch("superset.mcp_service.dataset.tool.create_dataset.CreateDatasetCommand") + @patch(_CMD_PATH) @pytest.mark.asyncio async def test_create_dataset_table_not_found(self, mock_command_class, mcp_server): - """Returns DatasetError when the physical table does not exist in the DB.""" - from superset.commands.dataset.exceptions import TableNotFoundValidationError + """Returns TableNotFoundError when the physical table does not exist in the DB. + + CreateDatasetCommand.validate() wraps TableNotFoundValidationError inside + DatasetInvalidError, so simulate the real command shape. + """ + from superset.commands.dataset.exceptions import ( + DatasetInvalidError, + TableNotFoundValidationError, + ) from superset.sql.parse import Table + exc = DatasetInvalidError() + exc.append(TableNotFoundValidationError(Table("missing_table", "public", None))) + mock_command = MagicMock() - mock_command.run.side_effect = TableNotFoundValidationError( - Table("missing_table", "public", None) - ) + mock_command.run.side_effect = exc mock_command_class.return_value = mock_command async with Client(mcp_server) as client: @@ -206,7 +225,7 @@ class TestCreateDataset: data = json.loads(result.content[0].text) assert data["error_type"] == "TableNotFoundError" - @patch("superset.mcp_service.dataset.tool.create_dataset.CreateDatasetCommand") + @patch(_CMD_PATH) @pytest.mark.asyncio async def test_create_dataset_unexpected_error( self, mock_command_class, mcp_server @@ -247,7 +266,7 @@ class TestCreateDataset: }, ) - @patch("superset.mcp_service.dataset.tool.create_dataset.CreateDatasetCommand") + @patch(_CMD_PATH) @pytest.mark.asyncio async def test_create_dataset_returns_full_dataset_info( self, mock_command_class, mcp_server
