codeant-ai-for-open-source[bot] commented on code in PR #44337:
URL: https://github.com/apache/superset/pull/44337#discussion_r4023724422
##########
superset/mcp_service/dataset/schemas.py:
##########
@@ -718,6 +739,237 @@ class UpdateDatasetMetricResponse(BaseModel):
)
+class DeleteDatasetRequest(BaseModel):
+ """Request schema for delete_dataset."""
+
+ identifier: int | str = Field(
+ ...,
+ description=(
+ "Dataset identifier - numeric ID or UUID string (NOT the table
name)."
+ ),
+ )
+
+ @field_validator("identifier", mode="before")
+ @classmethod
+ def reject_bool_identifier(cls, value: object) -> object:
+ """bool is a subclass of int, so identifier=true would coerce to
+ dataset ID 1 and delete the wrong object; reject it outright."""
+ if isinstance(value, bool):
+ raise ValueError("identifier must be an integer ID or UUID string")
+ return value
+
+
+class DeleteDatasetResponse(BaseModel):
+ """Result of a delete_dataset operation."""
+
+ success: bool = Field(description="Whether the dataset was deleted")
+ deleted_id: int | None = Field(None, description="ID of the deleted
dataset")
+ deleted_name: str | None = Field(
+ None, description="Table name of the deleted dataset"
+ )
+ soft_deleted: bool = Field(
+ False,
+ description=(
+ "True when the dataset was soft-deleted (moved to trash, because
the "
+ "SOFT_DELETE feature flag is enabled) and can be restored by an "
+ "owner or Admin. False means the delete was permanent."
+ ),
+ )
+ affected_chart_count: int = Field(
+ 0,
+ description=(
+ "Number of charts (visible to the caller) built on this dataset. "
+ "They stop working while the dataset is deleted."
+ ),
+ )
+ affected_dashboard_count: int = Field(
+ 0,
+ description=(
+ "Number of dashboards (visible to the caller) containing those
charts."
+ ),
+ )
+ message: str | None = Field(None, description="Human-readable outcome
message")
+ error: str | None = Field(None, description="Error message if the delete
failed")
+ error_type: str | None = Field(None, description="Type of error if failed")
+ permission_denied: bool = Field(
+ False,
+ description=(
+ "True when the caller lacks permission to delete the dataset (do
not "
+ "retry; ask the user)."
+ ),
+ )
+
+
+class RestoreDatasetRequest(BaseModel):
+ """Request schema for restore_dataset."""
+
+ identifier: int | str = Field(
+ ...,
+ description=(
+ "Dataset identifier - numeric ID or UUID string (NOT the table
name)."
+ ),
+ )
+
+ @field_validator("identifier", mode="before")
+ @classmethod
+ def reject_bool_identifier(cls, value: object) -> object:
+ """bool is a subclass of int, so identifier=true would coerce to
+ dataset ID 1 and target the wrong object; reject it outright."""
+ if isinstance(value, bool):
+ raise ValueError("identifier must be an integer ID or UUID string")
+ return value
+
+
+class RestoreDatasetResponse(BaseModel):
+ """Result of a restore_dataset operation."""
+
+ success: bool = Field(description="Whether the dataset was restored from
trash")
+ restored_id: int | None = Field(None, description="ID of the restored
dataset")
+ restored_name: str | None = Field(
+ None, description="Table name of the restored dataset"
+ )
+ message: str | None = Field(None, description="Human-readable outcome
message")
+ error: str | None = Field(None, description="Error message if the restore
failed")
+ error_type: str | None = Field(None, description="Type of error if failed")
+ permission_denied: bool = Field(
+ False,
+ description=(
+ "True when the caller lacks permission to restore the dataset (do
not "
+ "retry; ask the user)."
+ ),
+ )
+
+
+UPDATABLE_DATASET_FIELDS: frozenset[str] = frozenset(
+ {
+ "table_name",
+ "sql",
+ "description",
+ "main_dttm_col",
+ "cache_timeout",
+ }
+)
+
+
+class UpdateDatasetRequest(BaseModel):
+ """Request schema for update_dataset."""
+
+ model_config = ConfigDict(populate_by_name=True)
+
+ dataset_id: int | str = Field(
+ ...,
+ description="Dataset identifier โ numeric ID or UUID string. "
+ "Use list_datasets to find valid IDs.",
+ )
+ table_name: str | None = Field(
+ None,
+ max_length=250,
+ description="New dataset name. For a virtual dataset this is just its "
+ "label; for a physical dataset it must match an existing table.",
+ )
+ sql: str | None = Field(
+ None,
+ description="New SQL for a virtual dataset. Rejected for physical "
+ "datasets. Columns are re-synced from the new query unless "
+ "sync_columns is false.",
+ )
+ description: str | None = Field(None, description="Dataset description.")
+ main_dttm_col: str | None = Field(
+ None,
+ description="Default datetime column; must be one of the dataset's "
+ "columns (after re-sync, when columns are re-synced).",
+ )
+ cache_timeout: int | None = Field(
+ None,
+ ge=-1,
+ description="Cache timeout in seconds. 0 means the cache never
expires, "
+ "-1 bypasses the cache, null falls back to the database default.",
+ )
Review Comment:
**Suggestion:** `cache_timeout=true` is coerced to integer `1`, so invalid
boolean input silently changes the dataset's cache timeout. [type error]
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b507ddf720c84e9db895de7307d0b757&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=b507ddf720c84e9db895de7307d0b757&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<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:** 882:887
**Comment:**
*Type Error: `cache_timeout=true` is coerced to integer `1`, so invalid
boolean input silently changes the dataset's cache timeout.
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%2F44337&comment_hash=bb56012cf01a3643832740000e562ed7f57d748d7bd861be0ae7944dad044ab0&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44337&comment_hash=bb56012cf01a3643832740000e562ed7f57d748d7bd861be0ae7944dad044ab0&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]
