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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
