mikebridge commented on code in PR #45531:
URL: https://github.com/apache/superset/pull/45531#discussion_r4331544861
##########
superset/mcp_service/semantic_layer/tool/get_compatible_metrics.py:
##########
@@ -283,8 +283,9 @@ async def _resolve_compatible_metrics(
type(exc).__name__,
str(exc),
)
- await ctx.error("Unexpected error: %s: %s" % (type(exc).__name__,
str(exc)))
+ message: str = "Unable to retrieve compatible metrics."
Review Comment:
Good catch — 7caa2a7ded1be96c5c400a4d7bb19cafc9a7e6ac preserves
AmbiguousDimensionError as ValidationError in both compatibility tools, with
red-first coverage for column discovery and compatibility lookup. Generic
QueryObjectValidationError and provider failures still use the fixed message.
##########
superset/semantic_layers/api.py:
##########
@@ -306,14 +306,14 @@ def structure(self, pk: int) -> Response:
]
except SupersetSecurityException as ex:
return self.response(403, message=ex.message,
errors=[ex.to_dict()])
- except Exception as ex: # pylint: disable=broad-except
- logger.error(
- "Error fetching structure for semantic view %d: %s",
+ except Exception: # pylint: disable=broad-except
Review Comment:
Good catch; both handlers already lose the structured OAuth response on
master. I left this unchanged in 7caa2a7ded1be96c5c400a4d7bb19cafc9a7e6ac:
could we handle it separately with explicit OAuth passthrough and endpoint
tests, rather than passing through every SupersetErrorException subtype, which
can contain arbitrary text? That follow-up still needs a tracking issue; none
has been opened here.
--
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]
