codeant-ai-for-open-source[bot] commented on code in PR #44413:
URL: https://github.com/apache/superset/pull/44413#discussion_r4048159823
##########
superset/models/dashboard.py:
##########
@@ -346,21 +347,37 @@ def data(self) -> dict[str, Any]:
def datasets_trimmed_for_slices(
self,
- ) -> list[tuple[BaseDatasource, dict[str, Any]]]:
- slices_by_datasource: dict[int, set[Slice]] = defaultdict(set)
+ ) -> list[tuple[BaseDatasource | SemanticView, dict[str, Any]]]:
+ """Return trimmed chart metadata, keeping datasource types distinct."""
+ slices_by_datasource: dict[tuple[str, int], set[Slice]] =
defaultdict(set)
for slc in self.slices:
- slices_by_datasource[slc.datasource_id].add(slc)
+ slices_by_datasource[(slc.datasource_type,
slc.datasource_id)].add(slc)
- result: list[tuple[BaseDatasource, dict[str, Any]]] = []
+ result: list[tuple[BaseDatasource | SemanticView, dict[str, Any]]] = []
for _, slices in slices_by_datasource.items():
- # Use the eagerly-loaded datasource from any slice in the group
- datasource = next(iter(slices)).datasource
+ # Resolve once per typed datasource, retaining eager-loaded tables.
+ datasource: Datasource | None =
next(iter(slices)).resolved_datasource
- if datasource:
+ if isinstance(datasource, (BaseDatasource, SemanticView)):
# Filter out unneeded fields from the datasource payload
- result.append((datasource,
datasource.data_for_slices(list(slices))))
+ try:
+ payload: dict[str, Any] = dict(
+ datasource.data_for_slices(list(slices))
+ )
+ except Exception: # noqa: BLE001
+ if not isinstance(datasource, SemanticView):
+ raise
+ # Provider discovery must not hide other charts' metadata.
+ # Exception details may contain credentials or provider
URLs.
+ logger.warning(
+ "Could not serialize semantic view id=%s
layer_uuid=%s",
+ datasource.id,
+ datasource.semantic_layer_uuid,
+ )
+ continue
Review Comment:
Yes—the current broad catch is too permissive. Until the discovery layer
exposes a shared exception contract, the dashboard code cannot safely
distinguish provider failures from programming or malformed-metadata errors.
The preferred fix is to establish a semantic-layer-specific discovery
exception at the boundary:
```python
class SemanticProviderDiscoveryError(Exception):
"""Expected failure while discovering semantic-view metadata."""
```
Provider adapters should translate expected connection, authentication, and
discovery failures into this exception. `AttributeError`, schema/serialization
errors, and other unexpected exceptions should remain uncaught and propagate
normally.
Then narrow the dashboard handling:
```python
try:
payload = dict(datasource.data_for_slices(list(slices)))
except SemanticProviderDiscoveryError:
logger.warning(
"Could not serialize semantic view id=%s layer_uuid=%s",
datasource.id,
datasource.semantic_layer_uuid,
)
continue
```
The regression test should cover both behaviors:
- a `SemanticProviderDiscoveryError` omits only the failed semantic dataset
and preserves table metadata;
- an `AttributeError` or malformed payload error propagates.
Using CRUD command exceptions here would be incorrect because discovery has
a different failure boundary. This contract should be coordinated with the
provider/discovery ABC owners, after which the dashboard catch can be narrowed
safely.
--
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]
