hoon-e commented on PR #41404: URL: https://github.com/apache/superset/pull/41404#issuecomment-4828305298
@sha174n @rusackas Thanks both, this makes sense. I rechecked the dashboard MCP path and agree the fix should not live in the dashboard serializer copy. From the current code path, dashboard MCP does not appear to expose `roles`: `DashboardInfo` has no `roles` field, and `serialize_dashboard_object` does not populate roles. So I agree the dashboard-side `serialize_role_object` looks like leftover/unused code rather than the path causing this validation error. For this PR, I’d prefer not to delete that helper yet. Even if it appears unused in the current tree, removing it would be a separate cleanup with a slightly different impact surface, and I do not want to mix that into this regression fix. **I’ll roll back my dashboard-specific changes and leave the existing helper as-is.** Then I’ll redirect the actual fix to `superset/mcp_service/role/schemas.py`, where `RoleInfo.permissions` is intentionally populated by `get_role_info`. I’ll also keep `list_roles` from traversing permissions and add regression coverage for: - `list_roles` not touching `role.permissions` - `get_role_info` serializing FAB `PermissionView` objects as `"<permission> on <view>"` Once I push the update, I’ll include the exact validation results. What do you think about trying this? -- 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]
