bito-code-review[bot] commented on code in PR #44413:
URL: https://github.com/apache/superset/pull/44413#discussion_r4043752522
##########
superset/dashboards/api.py:
##########
@@ -756,6 +757,10 @@ def _serialize_dashboard_dataset(
) -> dict[str, Any]:
"""Dump a member dataset, narrowed when the caller cannot access it."""
serialized = self.dashboard_dataset_schema.dump(payload)
+ if isinstance(datasource, SemanticView):
+ # Redux keys dashboard datasets by Slice.form_data["datasource"],
+ # whereas SemanticView.uid is the provider's independent identity.
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Duplicated uid format string</b></div>
<div id="fix">
This inline f-string adds a third independent copy of the canonical
datasource uid format already defined in `BaseDatasource.uid`
(superset/connectors/sqla/models.py:362) and `superset/models/sql_lab.py:343`.
The comment correctly explains why `SemanticView.uid` (provider identity)
cannot be used, but a re-typed copy of the `{id}__{type}` convention can
silently diverge if the format ever changes, breaking dashboard dataset keying.
Prefer a shared property/helper for this format.
</div>
</div>
<small><i>Code Review Run #429390</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
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
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>CWE-755: Broad Catch Masks Errors</b></div>
<div id="fix">
The `except Exception` at `datasets_trimmed_for_slices` swallows every error
from `SemanticView.data_for_slices` (which returns `self.data`, including
provider discovery per `semantic_layers/models.py:696`), not just provider
failures. A code bug (TypeError/AttributeError) would be logged as 'Could not
serialize' and the semantic view silently omitted from the dashboard datasets
API (`dashboards/api.py:746`). Narrow the caught type to the provider failure
exception. ([CWE-755](https://cwe.mitre.org/data/definitions/755.html))
</div>
</div>
<small><i>Code Review Run #429390</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
