aminghadersohi commented on code in PR #45484:
URL: https://github.com/apache/superset/pull/45484#discussion_r4337599669
##########
superset/mcp_service/chart/compile.py:
##########
@@ -411,20 +411,35 @@ def _classify_as_database_error(exc: BaseException,
dataset_id: int) -> bool:
then falls back to the engine spec's ``extract_errors`` regex patterns —
the same classification the Superset UI uses.
"""
+ from superset.commands.chart.exceptions import (
+ ChartDataQueryFailedError,
+ ChartDataQueryValidationError,
+ )
+
+ # Validation wrappers can carry an annotation chart's database failure.
+ # Only a non-validation query failure may use the engine's generic
fallback.
+ database_exc = None if isinstance(exc, ChartDataQueryValidationError) else
exc
# Direct SQLAlchemy errors (unwrapped or in cause chain)
current: BaseException | None = exc
while current is not None:
if isinstance(current, SQLAlchemyError):
return True
+ if isinstance(current, ChartDataQueryFailedError) and not isinstance(
+ current, ChartDataQueryValidationError
+ ):
+ database_exc = current
current = current.__cause__
+ if database_exc is None:
+ return False
+
# Use the dataset's engine spec to classify (same as the UI)
try:
from superset.daos.dataset import DatasetDAO
dataset = DatasetDAO.find_by_id(dataset_id)
- if dataset and dataset.database and isinstance(exc, Exception):
- errors = dataset.database.db_engine_spec.extract_errors(exc)
+ if dataset and dataset.database and isinstance(database_exc,
Exception):
+ errors =
dataset.database.db_engine_spec.extract_errors(database_exc)
Review Comment:
Fixed in a63dc309672042cb9c26be1085e22072cbf3a907. The exception does not
name the annotation layer that raised it, so with annotation charts on several
engines the failing engine is unknown. `_annotation_engine_specs` now also
reports whether a single engine is certain (all chart layers resolved to the
same engine). When it is not, or when the dataset's spec is the fallback, only
specific connection patterns count and an engine's generic
`GENERIC_DB_ENGINE_ERROR` fallback no longer yields
`database_connection_error`; the connection message still says "annotation
layer's chart".
Regression test:
`tests/unit_tests/mcp_service/chart/test_compile.py::test_compile_chart_annotation_syntax_error_with_several_engines_is_not_connection`
(Postgres + MySQL annotation charts, MySQL syntax error). It failed at
e0991386a2e89e3a2c2814caf410f66f72869e5a (classified as
database_connection_error) and passes now.
`test_compile_chart_annotation_connection_error_with_several_engines` pins that
a specific connection pattern still classifies.
Siblings checked:
- superset/mcp_service/chart/compile.py:459 — fixed in
a63dc309672042cb9c26be1085e22072cbf3a907
- superset/mcp_service/chart/compile.py:534 — fixed in
a63dc309672042cb9c26be1085e22072cbf3a907
- superset/mcp_service/chart/compile.py:536 — fixed in
a63dc309672042cb9c26be1085e22072cbf3a907 (dataset-spec fallback for annotation
failures is also specific-only)
- superset/mcp_service/chart/compile.py:541 — not affected: main-dataset
failures keep the dataset spec with its generic fallback
- superset/common/query_context_processor.py:922 — not affected: NATIVE
annotation layers read the metadata database, not an external one
--
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]
