aminghadersohi commented on code in PR #40961:
URL: https://github.com/apache/superset/pull/40961#discussion_r3482505427
##########
superset/mcp_service/dashboard/schemas.py:
##########
@@ -376,6 +376,17 @@ class GetDashboardLayoutRequest(BaseModel):
]
+class GetDashboardDatasetsRequest(BaseModel):
+ """Request schema for get_dashboard_datasets."""
+
+ identifier: Annotated[
+ int | str,
+ Field(
+ description="Dashboard identifier - can be numeric ID, UUID
string, or slug"
+ ),
+ ]
Review Comment:
Keeping `int | str` for the identifier. Accepting numeric ID, UUID, or slug
is the explicit, documented design of this tool and matches the sibling
dashboard tools exactly (`GetDashboardInfoRequest`/`GetDashboardLayoutRequest`
both accept `int | str`). Access control is enforced during resolution (DAO
`base_filter` + `raise_for_access`), so accepting an integer id as input is not
an exposure. Restricting to UUID/slug-only would break consistency and remove a
documented capability.
##########
superset/mcp_service/dashboard/schemas.py:
##########
@@ -1512,3 +1523,225 @@ def dashboard_layout_serializer(dashboard: "Dashboard")
-> DashboardLayout:
has_layout=bool(position_json_str),
)
)
+
+
+# Per-dataset caps keep responses small enough for LLM context: wide
+# datasets can have hundreds of columns, which would dwarf the fields an
+# agent actually needs to configure native filters.
+MAX_DASHBOARD_DATASET_COLUMNS = 100
+MAX_DASHBOARD_DATASET_METRICS = 50
+
+
+class DashboardDatasetColumn(BaseModel):
+ """Lean column representation for dashboard dataset context."""
+
+ column_name: str = Field(..., description="Column name")
+ verbose_name: str | None = Field(None, description="Verbose (display)
name")
+ type: str | None = Field(None, description="Column data type")
+ is_dttm: bool | None = Field(None, description="Is datetime column")
+
+
+class DashboardDatasetMetric(BaseModel):
+ """Lean metric representation for dashboard dataset context."""
+
+ metric_name: str = Field(..., description="Saved metric name")
+ verbose_name: str | None = Field(None, description="Verbose (display)
name")
+ expression: str | None = Field(None, description="SQL expression")
+
+
+class DashboardDatasetDatabaseInfo(BaseModel):
+ """Database connection summary for a dashboard dataset."""
+
+ id: int | None = Field(None, description="Database ID")
Review Comment:
Same rationale as the dataset/dashboard `id` threads above (which were
accepted): the integer `id` is kept for consistency with `DatasetInfo` (which
exposes `database_id`) and the rest of the MCP schemas. It's the identifier
other tools key on. Not adding a UUID-only variant 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]
