aminghadersohi opened a new pull request, #44580:
URL: https://github.com/apache/superset/pull/44580
### SUMMARY
Add the two missing saved-metric operations to the MCP dataset tools:
- `create_dataset_metric`: identify a dataset by ID or UUID, supply a
required `metric_name` and `expression`, and optionally set every property
supported by `update_dataset_metric`. Returns the same `DatasetMetricDetail`
shape.
- `delete_dataset_metric`: identify a metric by ID, UUID, or name, remove
only that metric, and return its details plus accessible charts that reference
its name.
Investigation and implementation notes:
- Both tools use `Dataset` / `write` permissions, check dataset editorship
before inspecting metric names, and persist through `UpdateDatasetCommand`. Its
editorship, externally managed dataset, uniqueness, SQL-expression, and
transaction checks are not bypassed.
- The dataset DAO treats its metrics payload as a replacement list. Creation
preserves existing metrics as ID/name stubs and appends a new definition;
deletion omits only the target. Tests also exercise the real command/DAO,
including deleting the final metric.
- Creation shares writable-property validation with updates. Deletion reuses
the existing metric lookup and did-you-mean error. Duplicate names, missing
datasets/metrics, and permission failures have explicit errors.
- Chart impact reporting is scoped to the dataset and checks each chart's
access before returning its identity. It examines saved metric controls, HAVING
filters, and query contexts, not SQL-expression text; it does not rewrite
charts.
- Includes tool annotations, event logging, registration, cache exclusions,
committed-write response-size handling, and documentation alongside
`update_dataset_metric` (following #40975 and #43596).
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable: MCP tools only; no UI changes.
### TESTING INSTRUCTIONS
Automated validation:
```bash
pytest -q -rs \
tests/unit_tests/mcp_service/dataset \
tests/unit_tests/mcp_service/test_mcp_tool_registration.py \
tests/unit_tests/mcp_service/test_rbac_tool_enforcement.py \
tests/unit_tests/mcp_service/utils/test_response_size_utils.py
```
- Result: **408 passed, 1 skipped** (the optional `sqlalchemy_bigquery`
dialect is not installed). The focused new/update tool, registration, and RBAC
run also passed: **102/102**.
- Includes the existing `update_dataset_metric` tests, both new tools, real
command/DAO persistence, schema validation, chart-reference detection, and RBAC
denial through the registered MCP tool path.
- Pre-commit (including Ruff and mypy) run on all changed files; standalone
pylint run on the new tools and shared schemas.
- No live-server/UI smoke test: MCP behavior was exercised through the
in-process client.
Manual verification on a disposable dataset owned/editable by the caller:
1. Call `create_dataset_metric` with `{"dataset_id": 123, "metric_name":
"sum_revenue", "expression": "SUM(revenue)", "d3format": ",.2f"}`. Confirm the
returned ID/UUID and that existing metrics remain unchanged.
2. Repeat creation with the same name and verify the duplicate-name error.
Try a missing dataset and a caller without dataset write permission/editorship.
3. Save a chart referencing the metric. Call `delete_dataset_metric` with
`{"dataset_id": 123, "metric": "sum_revenue"}`. Confirm the metric is removed,
other metrics remain, and the chart appears in `affected_charts` without its
definition being rewritten.
4. Try an unknown metric name and verify did-you-mean guidance. Repeat using
metric IDs/UUIDs and dataset UUIDs.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [x] Introduces new feature or API
- [ ] Removes existing feature or API
--
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]
