aminghadersohi commented on code in PR #37183:
URL: https://github.com/apache/superset/pull/37183#discussion_r2775338423
##########
superset/mcp_service/chart/tool/get_chart_data.py:
##########
@@ -119,20 +144,102 @@ async def get_chart_data( # noqa: C901
)
logger.info("Getting data for chart %s: %s", chart.id,
chart.slice_name)
- import time
-
start_time = time.time()
+ # Track whether we're using unsaved state
+ using_unsaved_state = False
+ cached_form_data_dict = None
+
try:
await ctx.report_progress(2, 4, "Preparing data query")
from superset.charts.schemas import ChartDataQueryContextSchema
from superset.commands.chart.data.get_data_command import
ChartDataCommand
+ # Check if form_data_key is provided - use cached form_data instead
+ if request.form_data_key:
+ await ctx.info(
+ "Retrieving unsaved chart state from cache:
form_data_key=%s"
+ % (request.form_data_key,)
+ )
+ if cached_form_data :=
_get_cached_form_data(request.form_data_key):
+ try:
+ cached_form_data_dict =
utils_json.loads(cached_form_data)
+ using_unsaved_state = True
+ await ctx.info(
+ "Using cached form_data from form_data_key for
data query"
+ )
Review Comment:
Good catch — fixed in 6f0c01e. Now validates that the parsed form_data is
actually a dict before setting using_unsaved_state=True. If it's JSON null, a
list, or other non-dict type, we log a warning and fall back to saved chart
config.
##########
superset/mcp_service/dashboard/tool/get_dashboard_info.py:
##########
@@ -71,14 +110,67 @@ async def get_dashboard_info(
result = tool.run_tool(request.identifier)
if isinstance(result, DashboardInfo):
+ # If permalink_key is provided, retrieve filter state
+ if request.permalink_key:
+ await ctx.info(
+ "Retrieving filter state from permalink: permalink_key=%s"
+ % (request.permalink_key,)
+ )
+ permalink_value = _get_permalink_state(request.permalink_key)
+
+ if permalink_value:
+ # Verify the permalink belongs to the requested dashboard
+ # dashboardId in permalink is stored as str, result.id is
int
+ permalink_dashboard_id = permalink_value.get("dashboardId")
+ try:
+ permalink_dashboard_id_int = (
+ int(permalink_dashboard_id)
+ if permalink_dashboard_id
+ else None
+ )
+ except (ValueError, TypeError):
+ permalink_dashboard_id_int = None
+
+ if (
+ permalink_dashboard_id_int is not None
+ and permalink_dashboard_id_int != result.id
+ ):
+ await ctx.warning(
+ "permalink_key dashboardId (%s) does not match "
+ "requested dashboard id (%s); ignoring permalink "
+ "filter state." % (permalink_dashboard_id,
result.id)
+ )
+ else:
+ # Extract the state from permalink value
+ # Cast to dict for Pydantic compatibility
+ permalink_state = dict(permalink_value.get("state",
{}))
Review Comment:
Good catch — fixed in 6f0c01e. Now checks `isinstance(raw_state, dict)`
before calling `dict()` on it, falling back to empty dict `{}` if state is None
or non-dict.
--
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]
