tien238lnd commented on PR #44337: URL: https://github.com/apache/superset/pull/44337#issuecomment-5711541691
Addressed the review in a7739e4aa9, and rebased onto master. - `restore_dataset` checks editorship before any response that names the dataset (CodeAnt, Copilot, Bito). - `update_dataset` reports a refresh that `RefreshDatasetCommand` skips as a warning. This needed a `metadata_refreshed` flag on the command, the only change outside `superset/mcp_service/`. A failure while setting `main_dttm_col` after the re-sync is now a partial success, and `cache_timeout` rejects booleans. - `delete_dataset` runs the command's `validate()` (lookup and editorship) before counting dependent charts and dashboards (Bito). - The three tools are registered in the tool metadata test from #43828 and in `test_rbac_tool_enforcement.py` (Copilot). Two suggestions I left as they are: - Bito: `test_list_datasets_deleted_state.py` should expect `ValidationError` instead of `ToolError`. The server-side error type does not reach the test: `Client.call_tool` raises `ToolError` for any error result, and the test passes. - Copilot (suppressed comment): add `delete_dataset` and `restore_dataset` to the write-tools list in the server instructions. That list does not name `delete_chart`, `restore_chart`, `delete_dashboard` or `restore_dashboard` either; their descriptions say they need editor rights, and the dataset tools follow the same pattern. Listing all delete/restore tools there would fit better in a separate change. The commits on this branch also now carry my personal GitHub identity instead of a work address; the code is unchanged apart from the commit above. `pytest tests/unit_tests/mcp_service tests/unit_tests/commands/dataset` on this head: 4444 passed, 1 skipped, and 2 failures in `test_query_dataset.py` that fail the same way on master (4c77b0b924) in my environment. -- 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]
