You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "durgaprasadml (via GitHub)" <gi...@apache.org> on 2026/06/10 15:11:13 UTC
[PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
durgaprasadml opened a new pull request, #40935:
URL: https://github.com/apache/superset/pull/40935
Summary
This PR improves MCP parity with the Explore UI by enhancing support for adhoc SQL metrics and virtual dataset authoring workflows.
Changes Included
Virtual Dataset Enhancements
* Added support for:
* metrics
* calculated_columns
in CreateVirtualDatasetRequest
* Updated create_virtual_dataset flow to persist metrics and calculated columns using UpdateDatasetCommand
* Added handling for dataset update failures
* Added unit test coverage for metric and calculated column persistence
Chart Authoring Improvements
* Improved MCP chart-authoring guidance and examples for adhoc SQL metrics using sql_expression
* Added examples for:
* ratio metrics
* weighted averages
* conditional aggregates
* Updated chart schema example payloads to demonstrate valid adhoc SQL metric configurations
Notes
Backend support for adhoc SQL metrics already existed through existing metric validation and form_data handling paths.
This PR focuses on:
* improving MCP discoverability
* enabling end-to-end MCP authoring workflows
* improving LLM guidance/schema examples
* bringing MCP virtual dataset creation closer to Explore UI parity
The implementation preserves backward compatibility and reuses existing Explore-side metric handling mechanisms.
Testing
Added unit tests for:
* virtual dataset metric persistence
* calculated column persistence
* dataset update invocation flow
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "Madhu94 (via GitHub) via notifications" <no...@superset.apache.org>.
Madhu94 commented on PR #40935:
URL: https://github.com/apache/superset/pull/40935#issuecomment-6714706353
Link to #40917
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40935:
URL: https://github.com/apache/superset/pull/40935#issuecomment-4679328299
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40935:
URL: https://github.com/apache/superset/pull/40935#issuecomment-4674498234
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40935:
URL: https://github.com/apache/superset/pull/40935#issuecomment-4685334786
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "durgaprasadml (via GitHub)" <gi...@apache.org>.
durgaprasadml commented on PR #40935:
URL: https://github.com/apache/superset/pull/40935#issuecomment-4756706281
> The `schemas.py` refactor drops `CreateDatasetRequest` entirely, but `create_dataset.py` still imports it (line 39) — so mypy fails and ~30 MCP test modules error out at collection. Can you restore `CreateDatasetRequest` alongside the new `CreateDatasetMetric`/`CreateDatasetCalculatedColumn` classes? Also curious why the `.github/actions/setup-docker` and `github-action-validator.yml` changes ride along here — they seem unrelated to the MCP work.
Thanks for pointing that out.
I’ve restored CreateDatasetRequest and removed the unrelated CI workflow changes from this PR so the scope remains focused on the MCP virtual dataset metrics and adhoc SQL metric improvements.
The workflow changes were introduced while investigating CI failures during development, but I agree they are unrelated to the MCP functionality and don’t belong in this PR.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on code in PR #40935:
URL: https://github.com/apache/superset/pull/40935#discussion_r3446958340
##########
superset/mcp_service/dataset/tool/create_virtual_dataset.py:
##########
@@ -30,6 +30,58 @@
logger = logging.getLogger(__name__)
+def _build_update_props(
+ request: CreateVirtualDatasetRequest, dataset: Any
+) -> dict[str, Any]:
+ update_props: dict[str, Any] = {}
+ if request.metrics:
+ # Merge existing metrics with new ones
+ existing_metrics = [
+ {"id": m.id, "metric_name": m.metric_name} for m in dataset.metrics
+ ]
+ update_props["metrics"] = existing_metrics + [
+ m.model_dump(exclude_none=True) for m in request.metrics
+ ]
+ if request.calculated_columns:
+ # Merge existing columns with new ones
+ existing_cols = [
+ {"id": c.id, "column_name": c.column_name} for c in dataset.columns
+ ]
+ update_props["columns"] = existing_cols + [
+ c.model_dump(exclude_none=True) for c in request.calculated_columns
+ ]
+ return update_props
+
+
+def _cleanup_failed_dataset(dataset_id: int) -> None:
+ from superset.commands.dataset.delete import DeleteDatasetCommand
+
+ try:
+ DeleteDatasetCommand([dataset_id]).run()
+ except Exception as cleanup_exc:
+ logger.error(
+ "Failed to clean up dataset %s after update error: %s",
+ dataset_id,
+ cleanup_exc,
+ )
+
+
+def _update_virtual_dataset(dataset_id: int, update_props: dict[str, Any]) -> Any:
+ from superset.commands.dataset.exceptions import (
+ DatasetInvalidError,
+ DatasetUpdateFailedError,
+ )
+ from superset.commands.dataset.update import UpdateDatasetCommand
+
+ try:
+ return UpdateDatasetCommand(dataset_id, update_props).run()
+ except Exception as exc:
+ _cleanup_failed_dataset(dataset_id)
+ if not isinstance(exc, (DatasetUpdateFailedError, DatasetInvalidError)):
+ raise DatasetUpdateFailedError() from exc
+ raise
Review Comment:
[MEDIUM] `DatasetInvalidError` from `UpdateDatasetCommand` is re-raised here (update phase) and caught by the outer `except DatasetInvalidError` handler at line 170, which logs at `ctx.warning` level and returns `error=str(messages)` — no indication that a dataset was briefly created and deleted. The `DatasetUpdateFailedError` handler at line 193 already produces the correct caller-facing message (`"Failed to update dataset metadata (creation rolled back): {exc}"`). Route all update-phase failures through that handler by converting `DatasetInvalidError` here:
```python
except Exception as exc:
_cleanup_failed_dataset(dataset_id)
if not isinstance(exc, DatasetUpdateFailedError):
raise DatasetUpdateFailedError() from exc
raise
```
This also lets you remove `DatasetInvalidError` from the imports in `_update_virtual_dataset`. The missing companion test: add `mock_update_instance.run.side_effect = DatasetInvalidError()` as a second case in `test_create_virtual_dataset_update_failure_rollback` and assert `"creation rolled back" in data["error"]` — same assertion the `DatasetUpdateFailedError` case already makes.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "durgaprasadml (via GitHub)" <gi...@apache.org>.
durgaprasadml commented on PR #40935:
URL: https://github.com/apache/superset/pull/40935#issuecomment-4758903165
Thanks for the review and feedback.
I’ve addressed the requested changes:
* Added compensating cleanup for dataset update failures to avoid orphaned datasets.
* Improved error handling and update failure messaging.
* Added coverage for update failure paths and metrics-only / calculated-columns-only scenarios.
* Added schema docstrings and typing consistency updates.
* Removed unrelated CI workflow changes from this PR.
* Addressed Ruff, pre-commit, and code review comments (imports, payload shape, complexity, formatting, and test coverage).
All checks are now passing.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40935:
URL: https://github.com/apache/superset/pull/40935#issuecomment-4748913936
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40935:
URL: https://github.com/apache/superset/pull/40935#issuecomment-4671728540
<!-- Bito Reply -->
The flagged issue is correct. The `UpdateDatasetCommand` in Superset typically performs a full replacement of the `metrics` and `columns` collections. By passing only the new items from the request, existing columns or metrics inferred from the SQL query will be overwritten or deleted.
To resolve this, you should fetch the existing dataset's columns and metrics, merge them with the new items, and then pass the combined list to the `UpdateDatasetCommand`.
Here is the minimal fix for `superset/mcp_service/dataset/tool/create_virtual_dataset.py`:
```python
if request.metrics or request.calculated_columns:
from superset.commands.dataset.update import UpdateDatasetCommand
update_props: dict[str, Any] = {}
if request.metrics:
# Merge existing metrics with new ones
existing_metrics = [m.to_dict() for m in dataset.metrics]
update_props["metrics"] = existing_metrics + request.metrics
if request.calculated_columns:
# Merge existing columns with new ones
existing_cols = [c.to_dict() for c in dataset.columns]
update_props["columns"] = existing_cols + request.calculated_columns
with event_logger.log_context(action="mcp.create_virtual_dataset.update"):
dataset = UpdateDatasetCommand(dataset.id, update_props).run()
```
Would you like me to check the other comments on this PR and implement fixes for them as well?
**superset/mcp_service/dataset/tool/create_virtual_dataset.py**
```
if request.metrics or request.calculated_columns:
from superset.commands.dataset.update import UpdateDatasetCommand
update_props: dict[str, Any] = {}
if request.metrics:
# Merge existing metrics with new ones
existing_metrics = [m.to_dict() for m in dataset.metrics]
update_props["metrics"] = existing_metrics + request.metrics
if request.calculated_columns:
# Merge existing columns with new ones
existing_cols = [c.to_dict() for c in dataset.columns]
update_props["columns"] = existing_cols + request.calculated_columns
with event_logger.log_context(action="mcp.create_virtual_dataset.update"):
dataset = UpdateDatasetCommand(dataset.id, update_props).run()
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40935:
URL: https://github.com/apache/superset/pull/40935#discussion_r3398312067
##########
superset/mcp_service/dataset/tool/create_virtual_dataset.py:
##########
@@ -85,6 +86,37 @@ async def create_virtual_dataset(
dataset = CreateDatasetCommand(properties).run()
+ if request.metrics or request.calculated_columns:
+ from superset.commands.dataset.update import UpdateDatasetCommand
+
+ update_props: dict[str, Any] = {}
+ if request.metrics:
+ # Merge existing metrics with new ones
+ existing_metrics = (
+ [m.to_dict() for m in dataset.metrics]
+ if getattr(dataset, "metrics", None)
+ else []
+ )
+ update_props["metrics"] = existing_metrics + [
+ m.model_dump(exclude_none=True) for m in request.metrics
+ ]
+ if request.calculated_columns:
+ # Merge existing columns with new ones
+ existing_cols = (
+ [c.to_dict() for c in dataset.columns]
+ if getattr(dataset, "columns", None)
+ else []
+ )
+ update_props["columns"] = existing_cols + [
+ c.model_dump(exclude_none=True)
+ for c in request.calculated_columns
+ ]
+
+ with event_logger.log_context(
+ action="mcp.create_virtual_dataset.update"
+ ):
+ dataset = UpdateDatasetCommand(dataset.id, update_props).run()
Review Comment:
**Suggestion:** This introduces a two-step create-then-update flow without compensating cleanup, so if metric/column update fails, the dataset created in the first step is still persisted while the API returns an error. That leaves orphan/partial datasets and makes retries fail with dataset-name uniqueness conflicts. Make the operation atomic by wrapping both steps in one transaction boundary, or explicitly delete the newly created dataset when the update step raises. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ MCP virtual dataset tool reports failure but dataset actually created.
- ⚠️ Subsequent retries with same dataset_name fail due uniqueness.
- ⚠️ Leaves untracked datasets users did not intend to keep.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Call the MCP tool function `create_virtual_dataset` defined at
`superset/mcp_service/dataset/tool/create_virtual_dataset.py:43` with a
`CreateVirtualDatasetRequest` that includes an existing `database_id`, a new
`dataset_name`, valid `sql`, and a `metrics` list where two items share the same
`"metric_name"` (triggering duplicate-metric validation).
2. Inside the tool, the code at `create_virtual_dataset.py:75-87` builds `properties` and
calls `CreateDatasetCommand(properties).run()`, which validates uniqueness and permissions
and then persists the dataset within a transaction
(`superset/commands/dataset/create.py:46-52`), committing the new dataset row
successfully.
3. Because `request.metrics` is non-empty, the tool then constructs `update_props` and
calls `UpdateDatasetCommand(dataset.id, update_props).run()` at
`create_virtual_dataset.py:89-118`; during `UpdateDatasetCommand.validate()` the
`_validate_metrics` method (`superset/commands/dataset/update.py:14-31`) detects duplicate
`"metric_name"` values, appends `DatasetMetricsDuplicateValidationError`, and raises
`DatasetInvalidError`.
4. This `DatasetInvalidError` from the update step is caught by the `except
DatasetInvalidError` handler at `create_virtual_dataset.py:141-152`, which returns a
`CreateVirtualDatasetResponse` with `id=None`, `error=str(messages)`, and no cleanup of
the already-committed dataset, leaving an orphan dataset in the database; a later retry
with the same `dataset_name` re-enters `CreateDatasetCommand.validate()`
(`create.py:69-77`), which now fails `DatasetDAO.validate_uniqueness` and raises
`DatasetExistsValidationError` wrapped in `DatasetInvalidError`, so the tool again returns
an error while the original dataset remains in an unintended partial state.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8c1dd1b4e1014882a4d28e11df1131f9&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=8c1dd1b4e1014882a4d28e11df1131f9&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/dataset/tool/create_virtual_dataset.py
**Line:** 89:118
**Comment:**
*Incomplete Implementation: This introduces a two-step create-then-update flow without compensating cleanup, so if metric/column update fails, the dataset created in the first step is still persisted while the API returns an error. That leaves orphan/partial datasets and makes retries fail with dataset-name uniqueness conflicts. Make the operation atomic by wrapping both steps in one transaction boundary, or explicitly delete the newly created dataset when the update step raises.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40935&comment_hash=454a858b62f3b9d7fa5b513488855653c1a9de247de18dbbb0791a6eb2f534b0&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40935&comment_hash=454a858b62f3b9d7fa5b513488855653c1a9de247de18dbbb0791a6eb2f534b0&reaction=dislike'>👎</a>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #40935:
URL: https://github.com/apache/superset/pull/40935
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on code in PR #40935:
URL: https://github.com/apache/superset/pull/40935#discussion_r3445906542
##########
superset/mcp_service/dataset/tool/create_virtual_dataset.py:
##########
@@ -85,6 +86,54 @@ async def create_virtual_dataset(
dataset = CreateDatasetCommand(properties).run()
+ if request.metrics or request.calculated_columns:
+ from superset.commands.dataset.update import UpdateDatasetCommand
+
+ update_props: dict[str, Any] = {}
+ if request.metrics:
+ # Merge existing metrics with new ones
+ existing_metrics = (
+ [{"id": m.id} for m in dataset.metrics]
+ if getattr(dataset, "metrics", None)
Review Comment:
[N1 — outstanding] `getattr(dataset, "metrics", None)` is a redundant guard — `SqlaTable.metrics` is a declared ORM relationship and is always present. Same applies to `getattr(dataset, "columns", None)` at line 107. Simplify both to direct iteration without the conditional.
##########
superset/mcp_service/dataset/tool/create_virtual_dataset.py:
##########
@@ -85,6 +86,54 @@ async def create_virtual_dataset(
dataset = CreateDatasetCommand(properties).run()
+ if request.metrics or request.calculated_columns:
+ from superset.commands.dataset.update import UpdateDatasetCommand
Review Comment:
[N2 — outstanding] `UpdateDatasetCommand` is imported inside the `if request.metrics or request.calculated_columns` block. Move it to the outer `try` block alongside the existing lazy imports at lines 64–70.
##########
superset/mcp_service/dataset/tool/create_virtual_dataset.py:
##########
@@ -85,6 +86,54 @@ async def create_virtual_dataset(
dataset = CreateDatasetCommand(properties).run()
+ if request.metrics or request.calculated_columns:
+ from superset.commands.dataset.update import UpdateDatasetCommand
+
+ update_props: dict[str, Any] = {}
+ if request.metrics:
+ # Merge existing metrics with new ones
+ existing_metrics = (
+ [{"id": m.id} for m in dataset.metrics]
+ if getattr(dataset, "metrics", None)
+ else []
+ )
+ update_props["metrics"] = existing_metrics + [
+ m.model_dump(exclude_none=True) for m in request.metrics
+ ]
+ if request.calculated_columns:
+ # Merge existing columns with new ones
+ existing_cols = (
+ [{"id": c.id} for c in dataset.columns]
+ if getattr(dataset, "columns", None)
+ else []
+ )
+ update_props["columns"] = existing_cols + [
+ c.model_dump(exclude_none=True)
+ for c in request.calculated_columns
+ ]
+
+ with event_logger.log_context(
+ action="mcp.create_virtual_dataset.update"
+ ):
+ try:
+ dataset = UpdateDatasetCommand(dataset.id, update_props).run()
+ except Exception as exc:
+ from superset.commands.dataset.delete import DeleteDatasetCommand
Review Comment:
[NIT] `DeleteDatasetCommand` is imported inside the `except` block, the most deeply nested point in the function. Moving it to line 90 alongside `UpdateDatasetCommand` keeps cleanup and main command imports co-located and avoids a path where `ImportError` shadows the original exception.
##########
superset/mcp_service/dataset/tool/create_virtual_dataset.py:
##########
@@ -85,6 +86,54 @@ async def create_virtual_dataset(
dataset = CreateDatasetCommand(properties).run()
+ if request.metrics or request.calculated_columns:
+ from superset.commands.dataset.update import UpdateDatasetCommand
+
+ update_props: dict[str, Any] = {}
+ if request.metrics:
+ # Merge existing metrics with new ones
+ existing_metrics = (
+ [{"id": m.id} for m in dataset.metrics]
Review Comment:
[MEDIUM] `{"id": m.id}` passes only the primary key for each pre-existing metric to `UpdateDatasetCommand`. Whether the DAO treats an id-only entry as "preserve existing values unchanged" depends on the Marshmallow metric schema. If `metric_name` is required on all items (not just new ones), validation raises `DatasetInvalidError` at line 119 → cleanup runs → dataset is deleted with a misleading error. Verify by reading `DatasetDAO.update` / the metric Marshmallow schema, or add a test that exercises a pre-existing metric through this path.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #40935:
URL: https://github.com/apache/superset/pull/40935#issuecomment-4782656196
@aminghadersohi is on PTO, so I'll just merge this... we can revert/fix-forward if needed :)
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40935:
URL: https://github.com/apache/superset/pull/40935#discussion_r3389489799
##########
superset/mcp_service/dataset/schemas.py:
##########
@@ -444,6 +444,16 @@ class CreateVirtualDatasetRequest(BaseModel):
None,
description="Human-readable description of the dataset (optional).",
)
+ metrics: List[Dict[str, Any]] | None = Field(
+ None,
+ description="Optional list of saved metrics to create. Each dictionary "
+ "must have 'metric_name' and 'expression'.",
+ )
+ calculated_columns: List[Dict[str, Any]] | None = Field(
+ None,
+ description="Optional list of calculated columns to create. Each dictionary "
+ "must have 'column_name' and 'expression'.",
+ )
Review Comment:
**Suggestion:** `metrics` and `calculated_columns` accept arbitrary dictionaries without required-key validation, but downstream update validation assumes keys like `metric_name`/`column_name` exist and can raise uncaught `KeyError`, causing a 500. Add explicit typed item schemas or validators that enforce required fields before command execution. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ MCP virtual dataset creation can 500 on bad metrics.
- ⚠️ LLM-generated payloads may intermittently hit uncaught KeyError.
- ⚠️ Clients receive no structured field-level validation feedback.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Invoke the MCP tool `create_virtual_dataset` (implemented in
`superset/mcp_service/dataset/tool/create_virtual_dataset.py:4-82`) via the MCP server
with a `CreateVirtualDatasetRequest` payload.
2. Construct the request so that `request.metrics` contains at least one dict missing
`metric_name`, e.g. `metrics=[{"expression": "SUM(col1)"}]`, while other required fields
(`database_id`, `sql`, `dataset_name`) are valid (schema at
`superset/mcp_service/dataset/schemas.py:14-57`).
3. Inside `create_virtual_dataset`, after `CreateDatasetCommand(properties).run()`
succeeds (see `superset/commands/dataset/create.py:42-52`), the code at
`create_virtual_dataset.py:50-60` calls `UpdateDatasetCommand(dataset.id, {"metrics":
request.metrics}).run()`.
4. `UpdateDatasetCommand.run()` (`superset/commands/dataset/update.py:83-97`) calls
`self.validate()`, which in `_validate_semantics()` (`update.py:214-224`) calls
`_validate_metrics(metrics, exceptions)`; `_validate_metrics` calls
`_get_duplicates(metrics, "metric_name")` (`update.py:26-30, 86-93`), which evaluates
`[item[key] for item in data]` and raises a `KeyError` because `metric_name` is missing.
This `KeyError` is not caught by the transaction decorator (which only handles
`SQLAlchemyError`/`ValueError`) nor by the specific
`DatasetInvalidError`/`DatasetCreateFailedError`/`DatasetUpdateFailedError` handlers in
`create_virtual_dataset.py:83-105`, so it falls through to the generic `except Exception`
at `create_virtual_dataset.py:106-111` and is re-raised, causing the MCP tool call to fail
with an unhandled server error instead of a structured validation error.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=38b06c53d245440998004a3247cf9cbd&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=38b06c53d245440998004a3247cf9cbd&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/dataset/schemas.py
**Line:** 447:456
**Comment:**
*Api Mismatch: `metrics` and `calculated_columns` accept arbitrary dictionaries without required-key validation, but downstream update validation assumes keys like `metric_name`/`column_name` exist and can raise uncaught `KeyError`, causing a 500. Add explicit typed item schemas or validators that enforce required fields before command execution.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40935&comment_hash=cfb28253c76c8832a5a3711ebc2cd4d1470302077ce2bdae78f5b4d3dcb3afb3&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40935&comment_hash=cfb28253c76c8832a5a3711ebc2cd4d1470302077ce2bdae78f5b4d3dcb3afb3&reaction=dislike'>👎</a>
##########
superset/mcp_service/dataset/tool/create_virtual_dataset.py:
##########
@@ -85,6 +86,18 @@ async def create_virtual_dataset(
dataset = CreateDatasetCommand(properties).run()
+ if request.metrics or request.calculated_columns:
+ from superset.commands.dataset.update import UpdateDatasetCommand
+
+ update_props: dict[str, Any] = {}
+ if request.metrics:
+ update_props["metrics"] = request.metrics
Review Comment:
**Suggestion:** Passing only new `metrics` to `UpdateDatasetCommand` can remove pre-existing dataset metrics, since update semantics replace the metric set rather than append to it. Include existing metrics in the update payload (plus new ones) or use an append-specific path. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Auto-generated default metrics silently removed from new datasets.
- ⚠️ Any pre-existing saved metrics are fully replaced.
- ⚠️ MCP clients cannot rely on default metric availability.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Use the MCP `create_virtual_dataset` tool
(`superset/mcp_service/dataset/tool/create_virtual_dataset.py:4-82`) with a
`CreateVirtualDatasetRequest` that produces a valid dataset (so `CreateDatasetCommand`
succeeds) and provide `metrics` containing only the new logical metrics you want to add,
e.g. `[{"metric_name": "m1", "expression": "SUM(col1)"}]`, while the database layer is
configured to auto-populate default metrics via `SqlaTable.fetch_metadata()`
(`superset/connectors/sqla/models.py:11-28, 31-43, 55-15`).
2. After `CreateDatasetCommand(properties).run()`
(`superset/commands/dataset/create.py:42-52`) and `dataset.fetch_metadata()`, the dataset
has an initial set of metrics (derived from `database.get_metrics(...)` and
`add_missing_metrics`, `models.py:18-27, 31-15`), e.g. default `count` metrics.
3. Because `request.metrics` is truthy, `create_virtual_dataset` executes the block at
`create_virtual_dataset.py:50-60`, building `update_props["metrics"] = request.metrics`
(containing only the new metric definitions) and calling `UpdateDatasetCommand(dataset.id,
update_props).run()`.
4. Inside `UpdateDatasetCommand.run()` (`superset/commands/dataset/update.py:83-97`),
`DatasetDAO.update(self._model, attributes=self._properties)` is invoked;
`DatasetDAO.update` (`superset/daos/dataset.py:20-32`) sees `"metrics"` and calls
`update_metrics(model, attributes.pop("metrics"))`. `update_metrics`
(`dataset.py:114-158`) bulk-inserts metrics without `id` and then deletes all existing
metrics whose ids are not present in `property_metrics_by_id` (empty for new-only
metrics), thereby removing any pre-existing metrics (including auto-populated ones), so
the final dataset retains only the newly submitted `metrics` list.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=05cf9400f6fe495db712083e9180f31e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=05cf9400f6fe495db712083e9180f31e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/dataset/tool/create_virtual_dataset.py
**Line:** 93:94
**Comment:**
*Logic Error: Passing only new `metrics` to `UpdateDatasetCommand` can remove pre-existing dataset metrics, since update semantics replace the metric set rather than append to it. Include existing metrics in the update payload (plus new ones) or use an append-specific path.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40935&comment_hash=922b7dea5def2133e6a91942ece82cb9c8f2da726460665f9fce223edf6ce954&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40935&comment_hash=922b7dea5def2133e6a91942ece82cb9c8f2da726460665f9fce223edf6ce954&reaction=dislike'>👎</a>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40935:
URL: https://github.com/apache/superset/pull/40935#discussion_r3433606079
##########
superset/mcp_service/dataset/tool/create_virtual_dataset.py:
##########
@@ -118,8 +150,8 @@ async def create_virtual_dataset(
url=None,
error=str(messages),
)
- except DatasetCreateFailedError as exc:
- await ctx.error("Virtual dataset creation failed: %s" % (str(exc),))
+ except (DatasetCreateFailedError, DatasetUpdateFailedError) as exc:
Review Comment:
**Suggestion:** The shared failure path for create and update always reports "Failed to create dataset", which is incorrect for update failures. This misclassifies operational errors and can trigger wrong retry behavior and debugging decisions; return an update-specific message when the update command fails. [incorrect variable usage]
<details>
<summary><b>Severity Level:</b> Minor 🧹</summary>
```mdx
- ⚠️ Update failures misreported as create failures in responses.
- ⚠️ MCP clients cannot distinguish create vs update issues.
- ⚠️ Operators may debug wrong phase of dataset workflow.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Invoke `create_virtual_dataset` from
`superset/mcp_service/dataset/tool/create_virtual_dataset.py:43-139` with a valid request
including `database_id`, `sql`, `dataset_name`, and metrics/calculated columns so that the
create step succeeds.
2. Allow `CreateDatasetCommand(properties).run()` at `create_virtual_dataset.py:75-87` to
complete successfully (transaction defined in
`superset/commands/dataset/create.py:46-52`), committing the base dataset.
3. Trigger an operational failure in `UpdateDatasetCommand(dataset.id,
update_props).run()` at `create_virtual_dataset.py:89-118` (for example, by causing a
`SQLAlchemyError` inside the update transaction handled at
`superset/commands/dataset/update.py:83-91`), which is wrapped and raised as
`DatasetUpdateFailedError`.
4. Observe that the exception is caught by `except (DatasetCreateFailedError,
DatasetUpdateFailedError) as exc:` at `create_virtual_dataset.py:153-163`, where the log
line correctly says `"Virtual dataset creation/update failed"` but the response `error`
field is always set to `f"Failed to create dataset: {exc}"`, mislabeling this update
failure as a create failure for the MCP caller.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=437782aa65d046aabc27188659f7a342&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=437782aa65d046aabc27188659f7a342&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/dataset/tool/create_virtual_dataset.py
**Line:** 153:162
**Comment:**
*Incorrect Variable Usage: The shared failure path for create and update always reports "Failed to create dataset", which is incorrect for update failures. This misclassifies operational errors and can trigger wrong retry behavior and debugging decisions; return an update-specific message when the update command fails.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40935&comment_hash=1983ea31685ad29a8ec1e6c8df548ca18a289e5b233a29e78e606d501e7eb1da&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40935&comment_hash=1983ea31685ad29a8ec1e6c8df548ca18a289e5b233a29e78e606d501e7eb1da&reaction=dislike'>👎</a>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "durgaprasadml (via GitHub)" <gi...@apache.org>.
durgaprasadml commented on PR #40935:
URL: https://github.com/apache/superset/pull/40935#issuecomment-4756742511
> **Code Review — PR #40935** HEAD SHA: `79fc88284963841f0122f23a1bc1d0a65616e622`
>
> ### Scan Coverage (changed Python files)
> Scan Result
> Inline imports 1 instance — new `UpdateDatasetCommand` import inside nested `if` block (line 88), follows existing file convention
> `any` types 0 violations in new code
> SQL injection / expression concatenation 0 new paths; `expression` fields go through `UpdateDatasetCommand.validate()` / `validate_stored_expression`
> RBAC decorator `@tool(tags=["mutate"], class_permission_name="Dataset", method_permission_name="write")` — correct
> Error handling completeness 2 orphan paths (see B1, H2 below)
> `f-string` / string formatting Clean conversion from `%`-formatting; no regressions
> Pydantic model typing New models typed correctly; 1 style inconsistency (see M3)
> Test behavioral coverage Missing failure-path and asymmetric tests (see M2)
> Remaining 12 scans 0 violations
> ### BLOCKER
> **B1 — Orphan dataset on update failure (`create_virtual_dataset.py`)**
>
> The two-step create-then-update flow at lines 85–116 has no compensating rollback. `CreateDatasetCommand.run()` runs inside its own `@transaction` decorator and commits immediately. If `UpdateDatasetCommand` subsequently fails, the caller receives `id=None` and an error message, but the dataset already exists in the database.
>
> **Path A** — `except (DatasetCreateFailedError, DatasetUpdateFailedError)` at line 151: When `DatasetUpdateFailedError` is raised, the response returns `id=None` (line 153). The caller cannot distinguish "dataset was never created" from "dataset was created but metric/column update failed." On retry, `CreateDatasetCommand` raises `DatasetExistsValidationError` because the name is taken, permanently blocking clean creation with that name.
>
> **Path B** — `except DatasetInvalidError` at line 139: `UpdateDatasetCommand.validate()` can also raise `DatasetInvalidError` (e.g., invalid metric SQL expression that fails `validate_stored_expression`). This is caught by the same handler that covers SQL parse errors from the create phase. The response says "Virtual dataset validation failed" — indistinguishable from a SQL error — but the dataset IS persisted. The caller has no ID to clean up with.
>
> `CreateDatasetCommand` does not appear to accept `metrics`/`columns` directly (the command only processes `database`, `table_name`, `sql`, `schema`, `catalog`, `description`, `owners`), so a single-command path may not be available. Suggested fixes:
>
> 1. Split the except block: catch `DatasetUpdateFailedError` separately and return the created `id` alongside the error so the caller can clean up or accept the partial result.
> 2. Wrap both commands in a manual `db.session` transaction scope and roll back on update failure.
> 3. On `UpdateDatasetCommand` failure, call `DeleteDatasetCommand(dataset.id).run()` before returning the error response.
>
> Corroborates codeant findings #4 (id 3398301994, line 163) and #5 (id 3398312067, line 118), both still open. Path B (via `DatasetInvalidError`) is not covered by those comments.
>
> ### HIGH
> **H1 — Wrong error string for update failures (`create_virtual_dataset.py:160`)**
>
> ```python
> error=f"Failed to create dataset: {exc}",
> ```
>
> This fires for both `DatasetCreateFailedError` and `DatasetUpdateFailedError`. An LLM caller reading this message will incorrectly conclude the dataset was never created and will retry — producing the name-collision error from Path A above. At minimum, split the exception handlers or embed the phase in the message:
>
> ```python
> error=f"Dataset was created (id={dataset.id}) but metric/column update failed: {exc}",
> ```
>
> Corroborates codeant finding #8 (id 3433606079, line 153), still open.
>
> **H2 — `to_dict()` format compatibility with `UpdateDatasetCommand` input (`create_virtual_dataset.py:94,104`)**
>
> ```python
> existing_metrics = [m.to_dict() for m in dataset.metrics] # line 94
> existing_cols = [c.to_dict() for c in dataset.columns] # line 104
> ```
>
> `SqlaTable.metrics` and `SqlaTable.columns` are SQLAlchemy ORM objects. `to_dict()` on them returns dicts that include internal DB fields (`id`, `created_on`, `changed_on`, relationship back-references, etc.). `UpdateDatasetCommand.validate()` checks for duplicate metric names and column names against the existing model. Passing back metrics or columns that include their ORM `id` fields may cause the validation layer to treat them as update-in-place (correct) or raise `DatasetColumnsExistsValidationError` / `DatasetMetricsExistsValidationError` depending on how the validator resolves identity.
>
> For a freshly created dataset with no user-defined metrics, `dataset.metrics == []` and this is benign. But `dataset.columns` contains SQL-inferred columns immediately after `CreateDatasetCommand.run()`, so the `to_dict()` path IS exercised whenever `calculated_columns` is provided. Verify that passing inferred columns with their `id` fields in the update payload does not cause spurious duplicate-detection errors. If it does, filter out `id` before building `existing_cols`.
>
> ### MEDIUM
> **M1 — Column relationship untouched when only `metrics` is updated (`create_virtual_dataset.py:87–116`)**
>
> When `request.metrics` is set and `request.calculated_columns` is `None`, `update_props` only contains the `"metrics"` key. `UpdateDatasetCommand` is invoked without a `"columns"` key. Whether `DatasetDAO.update` with `attributes={"metrics": [...]}` leaves the `columns` relationship intact depends on the DAO implementation. If the DAO unconditionally processes all known relationship keys (clearing absent ones), SQL-inferred columns are silently deleted. Verify and add a test for the metrics-only path.
>
> **M2 — Missing test coverage for failure and asymmetric paths (`test_dataset_tools.py`)**
>
> `test_create_virtual_dataset_with_metrics_and_columns` covers the success case only, with both fields set. Missing tests:
>
> * `UpdateDatasetCommand` raises `DatasetUpdateFailedError` — documents the orphan-ID behavior for B1/Path A
> * `UpdateDatasetCommand` raises `DatasetInvalidError` — documents the wrong-handler path for B1/Path B
> * Only `metrics` provided, `calculated_columns` omitted — verifies columns relationship is untouched (M1)
> * Only `calculated_columns` provided, `metrics` omitted — symmetric check
>
> **M3 — `List[...]` vs `list[...]` inconsistency (`schemas.py:537,542`)**
>
> ```python
> metrics: List[CreateDatasetMetric] | None = Field(...) # line 537
> calculated_columns: List[CreateDatasetCalculatedColumn] | None # line 542
> ```
>
> The file imports `List` from `typing` (line 25) and uses it here, while the rest of the file uses the Python 3.9+ lowercase `list[...]` syntax (e.g., `list[str]`). Change to `list[CreateDatasetMetric] | None` and `list[CreateDatasetCalculatedColumn] | None`. Remove `List` from the `from typing import ...` line if it becomes unused after this change.
>
> **M4 — `security-events: write` permission added without explanation (`.github/workflows/github-action-validator.yml:14`)**
>
> Adding `security-events: write` grants the workflow permission to upload SARIF results to the repository Security tab. The PR description does not mention this change or identify which step requires it. If this is for a newly added CodeQL/Trivy scan, add an inline comment. If it is unrelated to this PR's intent, move it to a dedicated CI PR so the security permission grant is visible on its own.
>
> ### NIT
> **N1 — Redundant `getattr` defensive guards (`create_virtual_dataset.py:93–96,103–106`)**
>
> `SqlaTable.metrics` and `SqlaTable.columns` are declared SQLAlchemy relationships and always exist on the object. `getattr(dataset, "metrics", None)` is equivalent to `dataset.metrics`. The truthiness check (`if getattr(dataset, "metrics", None)`) also evaluates identically to `if dataset.metrics`, which itself is unnecessary since `[m.to_dict() for m in []]` already returns `[]`. Simplify to:
>
> ```python
> existing_metrics = [m.to_dict() for m in dataset.metrics]
> existing_cols = [c.to_dict() for c in dataset.columns]
> ```
>
> **N2 — `UpdateDatasetCommand` import placed inside nested `if` block (`create_virtual_dataset.py:88`)**
>
> The other three imports in this function are at the first level inside `try`. `UpdateDatasetCommand` is additionally nested inside `with event_logger.log_context(...)` and `if request.metrics or request.calculated_columns:`. Move it to the top of the `try` block alongside the other lazy imports.
>
> **N3 — Missing class docstrings on new Pydantic models (`schemas.py:413,423`)**
>
> `CreateDatasetMetric` (line 413) and `CreateDatasetCalculatedColumn` (line 423) have no class-level docstrings. Corroborates codeant findings #6 (id 3433547167) and #7 (id 3433547174), both open.
>
> ### Codeant finding status
> Comment ID Summary Status
> 3398301994 Orphan dataset on update failure **OPEN** — corroborated by B1/Path A
> 3398312067 Orphan/retry collision on two-step **OPEN** — corroborated by B1/Path A
> 3433606079 Wrong error string for update failures **OPEN** — corroborated by H1
> 3433547167 Missing docstring on `CreateDatasetMetric` **OPEN** — corroborated by N3
> 3433547174 Missing docstring on `CreateDatasetCalculatedColumn` **OPEN** — corroborated by N3
> 3389475131 UpdateDatasetCommand replaces not appends **NOT VALID** — code explicitly merges `dataset.metrics` with new metrics at lines 93–100
> 3389489799 `metrics`/`calculated_columns` accept arbitrary dicts without validation **NOT VALID** — `CreateDatasetMetric` and `CreateDatasetCalculatedColumn` are strongly-typed Pydantic models
> 3389489805 Passing only new metrics deletes existing ones **NOT VALID** — code merges existing metrics before calling UpdateDatasetCommand
> ### Non-MCP changes
> * `create_chart_guided.py` / `get_chart_type_schema.py`: The `sql_expression` + `label` guidance and example are correct and consistent with the Explore UI adhoc metric structure. The `saved_metric: true` JSON uses lowercase `true`. No issues.
> * `setup-docker/action.yml`: Adding `&& inputs.dockerhub-user != '' && inputs.dockerhub-token != ''` to the login condition is a defensive improvement. No issues.
Thanks for the detailed review.
The orphaned dataset scenario on update failure is a good catch. My original implementation assumed the create/update sequence would be treated as a single logical operation from the caller perspective, but since CreateDatasetCommand commits independently, a failed update can indeed leave behind a partially configured dataset.
I’ll review the available options (cleanup on failure vs. surfacing the created dataset id) and align the implementation with existing dataset command patterns before making changes.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40935:
URL: https://github.com/apache/superset/pull/40935#issuecomment-4762146303
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #40935:
URL: https://github.com/apache/superset/pull/40935#issuecomment-4671771979
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/40935?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `15.38462%` with `11 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 55.50%. Comparing base ([`0a1e51f`](https://app.codecov.io/gh/apache/superset/commit/0a1e51f542af034ba584dc9d57f760fb2cc8c800?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`e9f04b5`](https://app.codecov.io/gh/apache/superset/commit/e9f04b53ebc02106f7596bccda6bec8f06fed04d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/40935?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...mcp\_service/dataset/tool/create\_virtual\_dataset.py](https://app.codecov.io/gh/apache/superset/pull/40935?src=pr&el=tree&filepath=superset%2Fmcp_service%2Fdataset%2Ftool%2Fcreate_virtual_dataset.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbWNwX3NlcnZpY2UvZGF0YXNldC90b29sL2NyZWF0ZV92aXJ0dWFsX2RhdGFzZXQucHk=) | 0.00% | [11 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40935?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (0a1e51f) and HEAD (e9f04b5). Click for more details.
>
> <details><summary>HEAD has 109 uploads less than BASE</summary>
>
>| Flag | BASE (0a1e51f) | HEAD (e9f04b5) |
>|------|------|------|
>|python|86|2|
>|presto|13|1|
>|hive|14|1|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #40935 +/- ##
==========================================
- Coverage 64.17% 55.50% -8.67%
==========================================
Files 2654 2654
Lines 143763 143774 +11
Branches 33161 33164 +3
==========================================
- Hits 92260 79803 -12457
- Misses 49887 63254 +13367
+ Partials 1616 717 -899
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/40935/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/40935/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.47% <15.38%> (-0.01%)` | :arrow_down: |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/40935/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/40935/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/40935/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `41.06% <15.38%> (-0.01%)` | :arrow_down: |
| [python](https://app.codecov.io/gh/apache/superset/pull/40935/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `41.12% <15.38%> (-18.64%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/40935/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/40935/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/40935?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40935:
URL: https://github.com/apache/superset/pull/40935#issuecomment-4758908013
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40935:
URL: https://github.com/apache/superset/pull/40935#discussion_r3398301994
##########
superset/mcp_service/dataset/tool/create_virtual_dataset.py:
##########
@@ -131,7 +163,6 @@ async def create_virtual_dataset(
)
Review Comment:
**🟠 Architect Review — HIGH**
create_virtual_dataset now performs a CreateDatasetCommand followed by an UpdateDatasetCommand; if the update step raises DatasetInvalidError or DatasetUpdateFailedError, the function returns id=None and an error, but the dataset created in the first step has already been committed. This leaves a persisted "partial" dataset that the caller believes failed and makes subsequent retries with the same database/schema/catalog and dataset_name fail the uniqueness check ("dataset already exists").
**Suggestion:** Make the operation atomic from the tool caller's perspective by either wrapping create and update in a single transaction or compensating (e.g., deleting the newly created dataset) when the update fails, and adjust error handling to distinguish create vs update failures or return the created dataset id so callers can reconcile state.
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b249d6af40df4b5eb2e8634ffe832c4e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=b249d6af40df4b5eb2e8634ffe832c4e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.
**Path:** superset/mcp_service/dataset/tool/create_virtual_dataset.py
**Line:** 87:163
**Comment:**
*HIGH: create_virtual_dataset now performs a CreateDatasetCommand followed by an UpdateDatasetCommand; if the update step raises DatasetInvalidError or DatasetUpdateFailedError, the function returns id=None and an error, but the dataset created in the first step has already been committed. This leaves a persisted "partial" dataset that the caller believes failed and makes subsequent retries with the same database/schema/catalog and dataset_name fail the uniqueness check ("dataset already exists").
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "durgaprasadml (via GitHub)" <gi...@apache.org>.
durgaprasadml commented on PR #40935:
URL: https://github.com/apache/superset/pull/40935#issuecomment-4761727992
Thanks for catching that edge case.
I’ve updated _update_virtual_dataset() so that all update-phase failures, including DatasetInvalidError, are now routed through DatasetUpdateFailedError after cleanup. This ensures callers consistently receive the rollback-aware error path and don’t see a validation message that obscures the fact that the dataset was created and subsequently cleaned up.
I also added coverage for the DatasetInvalidError update-path in test_create_virtual_dataset_update_failure_rollback, verifying both cleanup behavior and the "creation rolled back" response message.
Changes are included in commit 4c74cea. All checks are passing.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40935:
URL: https://github.com/apache/superset/pull/40935#discussion_r3433547174
##########
superset/mcp_service/dataset/schemas.py:
##########
@@ -408,74 +410,24 @@ def _parse_column_fields(cls, value: Any) -> Any:
return parsed
-class CreateDatasetRequest(BaseModel):
- """Request schema for create_dataset to register a physical table as a dataset."""
-
- model_config = ConfigDict(populate_by_name=True)
-
- database_id: Annotated[
- int,
- Field(
- description="ID of the database connection to register the table against"
- ),
- ]
- schema_: Annotated[
- str | None,
- Field(
- default=None,
- alias="schema",
- serialization_alias="schema",
- max_length=250,
- description="Schema (namespace) where the table lives, e.g. 'public'. "
- "Omit or pass None for databases without schema namespaces (e.g. SQLite).",
- ),
- ]
- catalog: Annotated[
- str | None,
- Field(
- default=None,
- max_length=250,
- description="Catalog where the table lives. Omit for databases without "
- "catalog support.",
- ),
- ]
- table_name: Annotated[
- str,
- Field(
- min_length=1,
- max_length=250,
- description="Name of the physical table to register as a dataset",
- ),
- ]
- owners: Annotated[
- List[int] | None,
- Field(
- default=None,
- description="Optional list of owner user IDs. "
- "Defaults to the calling user.",
- ),
- ]
-
- @field_validator("schema_", "catalog", mode="before")
- @classmethod
- def _normalize_optional_str(cls, v: object) -> object:
- """Strip whitespace and convert blank strings to None.
+class CreateDatasetMetric(BaseModel):
+ metric_name: str = Field(..., description="Name of the metric")
+ expression: str = Field(..., description="SQL expression for the metric")
+ verbose_name: str | None = None
+ description: str | None = None
+ metric_type: str | None = None
+ d3format: str | None = None
+ warning_text: str | None = None
- Non-string values pass through unchanged so Pydantic's type validation
- rejects them, rather than silently treating a malformed value (e.g. an
- int or dict) as an omitted namespace.
- """
- if isinstance(v, str):
- return v.strip() or None
- return v
- @field_validator("table_name", mode="before")
- @classmethod
- def _strip_table_name(cls, v: object) -> object:
- """Strip leading/trailing whitespace from table_name."""
- if isinstance(v, str):
- return v.strip()
- return v
+class CreateDatasetCalculatedColumn(BaseModel):
Review Comment:
**Suggestion:** Add an inline class docstring that explains the role of this model and the semantics of the calculated column definition it captures. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The final file state shows this newly added class also has no docstring.
Since the rule says new Python classes should be documented inline, this is
a verified violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a13a5e33a98d458bb90372f762d48d27&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=a13a5e33a98d458bb90372f762d48d27&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/dataset/schemas.py
**Line:** 423:423
**Comment:**
*Custom Rule: Add an inline class docstring that explains the role of this model and the semantics of the calculated column definition it captures.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40935&comment_hash=36fe3fa68d99451f6b8690c0d249bd0c37b0205851176373814395ef47e8605b&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40935&comment_hash=36fe3fa68d99451f6b8690c0d249bd0c37b0205851176373814395ef47e8605b&reaction=dislike'>👎</a>
##########
superset/mcp_service/dataset/schemas.py:
##########
@@ -408,74 +410,24 @@ def _parse_column_fields(cls, value: Any) -> Any:
return parsed
-class CreateDatasetRequest(BaseModel):
- """Request schema for create_dataset to register a physical table as a dataset."""
-
- model_config = ConfigDict(populate_by_name=True)
-
- database_id: Annotated[
- int,
- Field(
- description="ID of the database connection to register the table against"
- ),
- ]
- schema_: Annotated[
- str | None,
- Field(
- default=None,
- alias="schema",
- serialization_alias="schema",
- max_length=250,
- description="Schema (namespace) where the table lives, e.g. 'public'. "
- "Omit or pass None for databases without schema namespaces (e.g. SQLite).",
- ),
- ]
- catalog: Annotated[
- str | None,
- Field(
- default=None,
- max_length=250,
- description="Catalog where the table lives. Omit for databases without "
- "catalog support.",
- ),
- ]
- table_name: Annotated[
- str,
- Field(
- min_length=1,
- max_length=250,
- description="Name of the physical table to register as a dataset",
- ),
- ]
- owners: Annotated[
- List[int] | None,
- Field(
- default=None,
- description="Optional list of owner user IDs. "
- "Defaults to the calling user.",
- ),
- ]
-
- @field_validator("schema_", "catalog", mode="before")
- @classmethod
- def _normalize_optional_str(cls, v: object) -> object:
- """Strip whitespace and convert blank strings to None.
+class CreateDatasetMetric(BaseModel):
Review Comment:
**Suggestion:** Add an inline class docstring describing the purpose of this request payload model and what kind of metric object it represents. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The final file state shows this newly added class has no docstring immediately
under the class definition. The custom rule requires newly added Python
classes to include docstrings, so this is a real violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e2581af2f62543b9aabb7f3900ecc8ee&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=e2581af2f62543b9aabb7f3900ecc8ee&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/dataset/schemas.py
**Line:** 413:413
**Comment:**
*Custom Rule: Add an inline class docstring describing the purpose of this request payload model and what kind of metric object it represents.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40935&comment_hash=d20f389485bb7d40c5c15237b02771b73db043899f4055d348215dfd82ddae4b&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40935&comment_hash=d20f389485bb7d40c5c15237b02771b73db043899f4055d348215dfd82ddae4b&reaction=dislike'>👎</a>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #40935:
URL: https://github.com/apache/superset/pull/40935#issuecomment-4747822941
The `schemas.py` refactor drops `CreateDatasetRequest` entirely, but `create_dataset.py` still imports it (line 39) — so mypy fails and ~30 MCP test modules error out at collection. Can you restore `CreateDatasetRequest` alongside the new `CreateDatasetMetric`/`CreateDatasetCalculatedColumn` classes? Also curious why the `.github/actions/setup-docker` and `github-action-validator.yml` changes ride along here — they seem unrelated to the MCP work.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40935:
URL: https://github.com/apache/superset/pull/40935#issuecomment-4757141237
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40935:
URL: https://github.com/apache/superset/pull/40935#discussion_r3389475131
##########
superset/mcp_service/dataset/tool/create_virtual_dataset.py:
##########
@@ -85,6 +86,18 @@ async def create_virtual_dataset(
dataset = CreateDatasetCommand(properties).run()
+ if request.metrics or request.calculated_columns:
+ from superset.commands.dataset.update import UpdateDatasetCommand
+
+ update_props: dict[str, Any] = {}
+ if request.metrics:
+ update_props["metrics"] = request.metrics
+ if request.calculated_columns:
+ update_props["columns"] = request.calculated_columns
+
Review Comment:
**🔴 Architect Review — CRITICAL**
The implementation assumes UpdateDatasetCommand will append calculated columns/metrics, but DatasetDAO.update_columns/update_metrics treat 'columns'/'metrics' as full replacement sets, so sending only request.calculated_columns or new metrics will delete existing dataset columns/metrics that were just inferred from SQL.
**Suggestion:** Build the UpdateDatasetCommand payloads by merging existing dataset.columns/metrics with the new entries (or use an additive path) before calling update; DatasetDAO.update_columns/update_metrics (superset/daos/dataset.py) show that unspecified existing items are deleted.
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9a306327463f4774b85492d641c5f4ee&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=9a306327463f4774b85492d641c5f4ee&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.
**Path:** superset/mcp_service/dataset/tool/create_virtual_dataset.py
**Line:** 89:97
**Comment:**
*CRITICAL: The implementation assumes UpdateDatasetCommand will append calculated columns/metrics, but DatasetDAO.update_columns/update_metrics treat 'columns'/'metrics' as full replacement sets, so sending only request.calculated_columns or new metrics will delete existing dataset columns/metrics that were just inferred from SQL.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40935:
URL: https://github.com/apache/superset/pull/40935#issuecomment-4671691104
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40935:
URL: https://github.com/apache/superset/pull/40935#issuecomment-4677751940
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(mcp): support virtual dataset metrics and improve adhoc SQL metric discoverability [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #40935:
URL: https://github.com/apache/superset/pull/40935#issuecomment-4683798129
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | c6dff474d7232af1aa396dcc9d8bc29ef860ee76 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a2afffc7561d600081df68f |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-40935--superset-docs-preview.netlify.app](https://deploy-preview-40935--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcpBCoAgEADAv-w5SYNM-kHPsNWlQNN0PUT097zNYV7IJcXMsAIMsBd74dFN3nIrfoyYBbbKKYp6BxE9lxNrn87nkJ7N9avtZIkIl1krp6WURjnShuD7AQ) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-40935--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org