bito-code-review[bot] commented on code in PR #40358:
URL: https://github.com/apache/superset/pull/40358#discussion_r3331103752
##########
superset/mcp_service/tag/schemas.py:
##########
@@ -249,3 +249,114 @@ def serialize_tag_object(tag: Any) -> TagInfo | None:
created_on_humanized=humanize_timestamp(getattr(tag, "created_on",
None)),
)
)
+
+
+class CreateTagRequest(BaseModel):
+ model_config = ConfigDict(populate_by_name=True)
+
+ name: str = Field(
+ ...,
+ min_length=1,
+ max_length=250,
+ description=(
+ "Name for the new tag. Must be unique. "
+ "Used to identify and reference the tag across Superset."
+ ),
+ )
+ description: str | None = Field(
+ None,
+ description="Optional human-readable description for the tag.",
+ )
+ objects_to_tag: list[tuple[str, int]] = Field(
+ default_factory=list,
+ description=(
+ "Optional list of objects to apply this tag to immediately after
creation. "
+ "Each item is a [object_type, object_id] pair where object_type is
one of: "
+ "'chart', 'dashboard', 'dataset', 'query'."
+ ),
+ )
+
+ @field_validator("name")
+ @classmethod
+ def name_must_not_be_blank(cls, v: str) -> str:
+ stripped = v.strip()
+ if not stripped:
+ raise ValueError("name must not be blank or whitespace-only")
+ return stripped
+
+ @field_validator("objects_to_tag")
+ @classmethod
+ def validate_object_ids(cls, v: list[tuple[str, int]]) -> list[tuple[str,
int]]:
+ for obj_type, obj_id in v:
+ if obj_id < 1:
+ raise ValueError(
+ f"object_id must be >= 1, got {obj_id} for type
'{obj_type}'"
+ )
+ return v
+
+
+class CreateTagResponse(BaseModel):
+ id: int | None = Field(
+ None,
+ description="ID of the created tag. None if creation failed.",
+ )
+ name: str = Field(..., description="Tag name.")
+ description: str | None = Field(None, description="Tag description.")
+ objects_tagged: list[tuple[str, int]] = Field(
+ default_factory=list,
+ description="Objects that were successfully tagged.",
+ )
+ objects_skipped: list[tuple[str, int]] = Field(
+ default_factory=list,
+ description=(
+ "Objects that were skipped because the current user lacks
ownership."
+ ),
+ )
+ error: str | None = Field(
+ None,
+ description="Error message if creation failed, otherwise null.",
+ )
+
+
+class UpdateTagRequest(BaseModel):
+ model_config = ConfigDict(populate_by_name=True)
+
+ id: int = Field(..., description="ID of the tag to update.")
+ name: str | None = Field(
+ None,
+ min_length=1,
+ max_length=250,
+ description=(
+ "New name for the tag. Omit to keep the existing name. "
+ "Must be unique across all tags."
+ ),
+ )
+ description: str | None = Field(
+ None,
+ description=(
+ "New description for the tag. Omit to keep the existing
description."
+ ),
+ )
+
+ @field_validator("name")
+ @classmethod
+ def name_must_not_be_blank(cls, v: str | None) -> str | None:
+ if v is not None:
+ stripped = v.strip()
+ if not stripped:
+ raise ValueError("name must not be blank or whitespace-only")
+ return stripped
+ return v
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incomplete object type validation</b></div>
<div id="fix">
The `validate_object_ids` validator only checks `obj_id >= 1` but does not
validate `obj_type` against the documented allowed values. Invalid `obj_type`
strings silently pass schema validation and cause errors at the command layer
that result in skipped objects with no clear error returned to the caller.
</div>
</div>
<div id="suggestion">
<div id="issue"><b>Semantic duplication of validator</b></div>
<div id="fix">
The `name_must_not_be_blank` validator body is semantically duplicated
across `CreateTagRequest` (lines 279-285) and `UpdateTagRequest` (lines
341-349). Duplication creates divergence risk if the validation logic is
updated for one class but not the other.
</div>
</div>
<small><i>Code Review Run #67db6f</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
