aminghadersohi commented on code in PR #37859:
URL: https://github.com/apache/superset/pull/37859#discussion_r2790156512
##########
superset/mcp_service/chart/tool/get_chart_preview.py:
##########
@@ -1807,66 +1808,72 @@ async def _get_chart_preview_internal( # noqa: C901
from superset.daos.chart import ChartDAO
# Find the chart
- chart: Any = None
- if isinstance(request.identifier, int) or (
- isinstance(request.identifier, str) and
request.identifier.isdigit()
- ):
- chart_id = (
- int(request.identifier)
- if isinstance(request.identifier, str)
- else request.identifier
- )
- await ctx.debug(
- "Performing ID-based chart lookup: chart_id=%s" % (chart_id,)
- )
- chart = ChartDAO.find_by_id(chart_id)
- else:
- await ctx.debug(
- "Performing UUID-based chart lookup: uuid=%s" %
(request.identifier,)
- )
- # Try UUID lookup using DAO flexible method
- chart = ChartDAO.find_by_id(request.identifier, id_column="uuid")
-
- # If not found and looks like a form_data_key, try to create
transient chart
- if (
- not chart
- and isinstance(request.identifier, str)
- and len(request.identifier) > 8
+ with
event_logger.log_context(action="mcp.get_chart_preview.chart_lookup"):
+ chart: Any = None
+ if isinstance(request.identifier, int) or (
+ isinstance(request.identifier, str) and
request.identifier.isdigit()
):
- # This might be a form_data_key, try to get form data from
cache
- from superset.commands.explore.form_data.get import
GetFormDataCommand
- from superset.commands.explore.form_data.parameters import (
- CommandParameters,
+ chart_id = (
+ int(request.identifier)
+ if isinstance(request.identifier, str)
+ else request.identifier
)
-
- try:
- cmd_params = CommandParameters(key=request.identifier)
- cmd = GetFormDataCommand(cmd_params)
- form_data_json = cmd.run()
- if form_data_json:
- from superset.utils import json as utils_json
-
- form_data = utils_json.loads(form_data_json)
-
- # Create a transient chart object from form data
- class TransientChart:
- def __init__(self, form_data: Dict[str, Any]):
- self.id = None
- self.slice_name = "Unsaved Chart Preview"
- self.viz_type = form_data.get("viz_type",
"table")
- self.datasource_id = None
- self.datasource_type = "table"
- self.params = utils_json.dumps(form_data)
- self.form_data = form_data
- self.uuid = None
-
- chart = TransientChart(form_data)
- except Exception as e:
- # Form data key not found or invalid
- logger.debug(
- "Failed to get form data for key %s: %s",
request.identifier, e
+ await ctx.debug(
+ "Performing ID-based chart lookup: chart_id=%s" %
(chart_id,)
+ )
+ chart = ChartDAO.find_by_id(chart_id)
+ else:
+ await ctx.debug(
+ "Performing UUID-based chart lookup: uuid=%s"
+ % (request.identifier,)
+ )
+ # Try UUID lookup using DAO flexible method
+ chart = ChartDAO.find_by_id(request.identifier,
id_column="uuid")
+
+ # If not found and looks like a form_data_key, try transient
+ if (
+ not chart
+ and isinstance(request.identifier, str)
+ and len(request.identifier) > 8
+ ):
+ # This might be a form_data_key
+ from superset.commands.explore.form_data.get import (
+ GetFormDataCommand,
+ )
+ from superset.commands.explore.form_data.parameters import
(
+ CommandParameters,
)
+ try:
+ cmd_params = CommandParameters(key=request.identifier)
+ cmd = GetFormDataCommand(cmd_params)
+ form_data_json = cmd.run()
+ if form_data_json:
+ from superset.utils import json as utils_json
+
+ form_data = utils_json.loads(form_data_json)
+
+ # Create a transient chart object from form data
+ class TransientChart:
+ def __init__(self, form_data: Dict[str, Any]):
+ self.id = None
+ self.slice_name = "Unsaved Chart Preview"
+ self.viz_type = form_data.get("viz_type",
"table")
+ self.datasource_id = None
+ self.datasource_type = "table"
+ self.params = utils_json.dumps(form_data)
+ self.form_data = form_data
+ self.uuid = None
+
+ chart = TransientChart(form_data)
+ except Exception as e:
Review Comment:
Fixed in 4ec8ba5. Replaced blind `except Exception` with `except
(ValueError, KeyError, AttributeError, TypeError)` to catch only the specific
exceptions that `GetFormDataCommand.run()`, JSON parsing, and dict/attribute
access can throw.
##########
superset/mcp_service/middleware.py:
##########
@@ -115,16 +117,76 @@ async def on_message(
user_id = get_user_id()
except Exception:
user_id = None
Review Comment:
Fixed in 4ec8ba5. Replaced `except Exception` with `except (RuntimeError,
AttributeError)` — these are the specific exceptions `get_user_id()` can throw
(RuntimeError when no Flask request context, AttributeError when `g.user` is
missing).
--
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]
