aminghadersohi commented on PR #40340: URL: https://github.com/apache/superset/pull/40340#issuecomment-4581685281
Thanks @richardfogaca for the thorough review! Here is what was addressed in commit `dea3f47285`: 1. **`app.py:540` — Initialization order** ✅ Restored `initialize_core_mcp_dependencies()` in `app.py` before the tool imports. The call was accidentally removed; without it a standalone `import superset.mcp_service.app` would hit `NotImplementedError` when the `@tool` stub fires. Back with a comment explaining the contract. 2. **`create_dataset.py:112` — Physical-table access check** — You are right that a user with `Dataset.write` but no table-level access can register arbitrary tables via MCP. The same gap exists in the REST API (`POST /api/v1/dataset/`) — `CreateDatasetCommand.validate()` only calls `raise_for_access` for the SQL/virtual path, not the physical-table path. Adding the check only in the MCP layer would create inconsistency. Tracking as a follow-up to `CreateDatasetCommand.validate()` so the REST API and MCP are fixed together. 3. **`create_dataset.py:126` — Unreachable exception handlers** ✅ You are correct — `CreateDatasetCommand.validate()` always wraps `DatasetExistsValidationError` and `TableNotFoundValidationError` into a single `DatasetInvalidError(exceptions=[...])` and never raises them directly. The handlers now inspect `exc.get_list_classnames()` inside the `DatasetInvalidError` branch so the typed `DatasetExistsError` / `TableNotFoundError` responses are actually reachable. Tests updated to use `DatasetInvalidError(exceptions=[...])` matching real command behavior. 4. **`schemas.py:336` — Missing `catalog` field** ✅ Added `catalog: str | None = None` to `CreateDatasetRequest` with the same blank-string normalization as `schema`, passed through to `CreateDatasetCommand` properties. -- 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]
