bito-code-review[bot] commented on code in PR #37859:
URL: https://github.com/apache/superset/pull/37859#discussion_r2790162093
##########
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:
<!-- Bito Reply -->
The suggestion replaces the broad 'except Exception' with specific
exceptions (ValueError, KeyError, AttributeError, TypeError) that match the
operations in the try block—GetFormDataCommand.run(), JSON parsing via
utils_json.loads(), and dictionary/attribute access—improving error handling by
avoiding unintended exception catches and making debugging easier.
**superset/mcp_service/chart/tool/get_chart_preview.py**
```
except (ValueError, KeyError, AttributeError, TypeError) as e:
```
--
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]
