aminghadersohi commented on code in PR #42659:
URL: https://github.com/apache/superset/pull/42659#discussion_r3716934098
##########
superset/mcp_service/dashboard/tool/get_dashboard_info.py:
##########
@@ -96,13 +96,57 @@ def _get_permalink_state(permalink_key: str) ->
DashboardPermalinkValue | None:
Returns the permalink value containing dashboardId and state if found,
None otherwise.
"""
- from superset.commands.dashboard.permalink.get import
GetDashboardPermalinkCommand
+ resolved = get_dashboard_permalink(permalink_key)
+ return resolved[1] if resolved else None
+
+
+def _lookup_dashboard(
+ tool: ModelGetInfoCore,
+ request: GetDashboardInfoRequest,
+) -> tuple[DashboardInfo | DashboardError, str | None, DashboardPermalinkValue
| None]:
+ """Resolve an ordinary identifier or dashboard permalink, then run
lookup."""
+ permalink_key = request.permalink_key
+ if isinstance(request.identifier, str):
+ extracted_key = extract_dashboard_permalink_key(request.identifier)
+ if extracted_key != request.identifier:
+ permalink_key = extracted_key
+ permalink_value = _get_permalink_state(permalink_key) if permalink_key
else None
+ if permalink_key and permalink_value is None:
+ return (
+ DashboardError.create(
+ "Dashboard permalink could not be resolved. It may be invalid
or "
+ "expired; ask for a fresh shared dashboard link.",
+ "permalink_not_found",
+ ),
+ permalink_key,
+ None,
+ )
+ if permalink_key:
+ permalink_key = extract_dashboard_permalink_key(permalink_key)
- try:
- return GetDashboardPermalinkCommand(permalink_key).run()
- except DashboardPermalinkGetFailedError as e:
- logger.warning("Failed to retrieve permalink state: %s", e)
- return None
+ lookup_identifier = (
+ permalink_value.get("dashboardId")
+ if permalink_value is not None
+ else request.identifier
+ )
+ result = tool.run_tool(lookup_identifier) # type: ignore[arg-type]
Review Comment:
Addressed. The shared lookup helper now preserves the existing contract:
when both fields are present, `identifier` selects the dashboard and
`permalink_key` only contributes state. If the permalink belongs to another
dashboard, its state is ignored. Regression coverage was added for both
`get_dashboard_info` and `get_dashboard_layout`.
--
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]