aminghadersohi commented on PR #40340:
URL: https://github.com/apache/superset/pull/40340#issuecomment-4585133124
Thanks for the detailed review, Richard! Addressing each point:
**1. `app.py:540` — init order** ✅ Fixed in `dea3f47285`
`initialize_core_mcp_dependencies()` is now called immediately after `mcp =
create_mcp_app()` and before any tool module imports. The `@tool` stub raises
`NotImplementedError` until injection runs, so this ordering is enforced by the
comment block and commit message.
**2. `create_dataset.py:112` — physical-table access check** ✅ Fixed in
`b44e9b9065` (just pushed)
Added an explicit pre-check before `CreateDatasetCommand`:
```python
database = DatabaseDAO.find_by_id(request.database_id)
if database is None:
return DatasetError.create(error=..., error_type="DatabaseNotFoundError")
table_obj = Table(table_name, schema, catalog)
try:
security_manager.raise_for_access(database=database, table=table_obj)
except SupersetSecurityException as exc:
return DatasetError.create(error=str(exc),
error_type="AccessDeniedError")
```
This mirrors the check `CreateDatasetCommand.validate()` already does for
the SQL/virtual path. New test coverage added for both the not-found and
access-denied paths.
**3. `create_dataset.py:126` — unreachable
`DatasetExistsValidationError`/`TableNotFoundValidationError` branches** ✅
Fixed in `dea3f47285`
The two direct `except` handlers were removed. The `DatasetInvalidError`
branch now calls `exc.get_list_classnames()` and maps to `DatasetExistsError` /
`TableNotFoundError` / `ValidationError` accordingly. Tests added in
`test_create_dataset_already_exists` and `test_create_dataset_table_not_found`.
**4. `schemas.py:336` — missing `catalog` field** ✅ Fixed in `dea3f47285`
`CreateDatasetRequest` now includes `catalog: str | None = None` with the
same blank-string normalization as `schema`. It's forwarded to
`CreateDatasetCommand` when non-null.
--
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]
