Copilot commented on code in PR #40746:
URL: https://github.com/apache/superset/pull/40746#discussion_r3364272849
##########
superset/mcp_service/user/schemas.py:
##########
@@ -104,6 +104,20 @@ class UserInfo(BaseModel):
"access via get_user_info; not available in list_users because roles "
"is a relationship, not a selectable column)",
)
+
+ @field_validator("roles", mode="before")
+ @classmethod
+ def _extract_role_names(cls, v: Any) -> list[str] | None:
+ """Coerce Role ORM objects to their .name strings."""
+ if v is None:
+ return None
+ result: list[str] = []
+ for item in v:
+ if isinstance(item, str):
+ result.append(item)
+ elif hasattr(item, "name"):
+ result.append(str(item.name))
+ return result if result else None
Review Comment:
The new `roles` pre-validator will iterate over a plain string input (e.g.
`roles="Admin"`) and return a list of characters, which changes prior behavior
(Pydantic would have rejected a string) and could silently corrupt the `roles`
field. It also converts an empty list of roles to `None` (`return result if
result else None`), conflating “no roles” with “roles not provided/redacted”.
Additionally, `item.name` can raise `DetachedInstanceError` for detached ORM
objects; the validator should handle this to avoid reintroducing serialization
failures.
##########
superset/mcp_service/user/schemas.py:
##########
@@ -104,6 +104,20 @@ class UserInfo(BaseModel):
"access via get_user_info; not available in list_users because roles "
"is a relationship, not a selectable column)",
)
+
+ @field_validator("roles", mode="before")
+ @classmethod
+ def _extract_role_names(cls, v: Any) -> list[str] | None:
Review Comment:
This change is meant to fix MCP tool failures when `UserInfo` is created via
`from_attributes` and the ORM returns `Role` objects. There are existing unit
tests for MCP user tools/schemas, but none cover this specific coercion path
(e.g. `UserInfo.model_validate(user_obj, from_attributes=True)` with
`user_obj.roles` as Role-like objects). Adding a focused unit test would
prevent regressions and validate the reported bug scenario.
--
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]
