codeant-ai-for-open-source[bot] commented on code in PR #40008:
URL: https://github.com/apache/superset/pull/40008#discussion_r3216302331
##########
superset/commands/dashboard/importers/v1/utils.py:
##########
@@ -139,6 +145,18 @@ def update_id_refs( # pylint: disable=too-many-locals #
noqa: C901
native_filter["scope"]["excluded"] = [
id_map[old_id] for old_id in scope_excluded if old_id in id_map
]
+
+ # fix display control dataset references
+ chart_customization_config = fixed.get("metadata", {}).get(
+ "chart_customization_config", []
+ )
+ for customization in chart_customization_config:
+ targets = customization.get("targets", [])
+ for target in targets:
+ dataset_uuid = target.pop("datasetUuid", None)
+ if dataset_uuid:
+ target["datasetId"] =
dataset_info[dataset_uuid]["datasource_id"]
Review Comment:
**Suggestion:** This new display-control remapping path can raise a
`KeyError` when a dashboard references a `datasetUuid` that is not present in
`dataset_info` (for example, missing dataset files, failed dataset import, or
partial bundles). That turns import into an unhandled runtime crash instead of
a controlled import error. Guard the lookup before indexing (or raise
`ImportFailedError` with context) so missing dataset mappings are handled
deterministically. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Dashboard ZIP import crashes on missing display-control dataset.
- ⚠️ Display controls fail when dataset_info mapping incomplete.
- ⚠️ Users lack clear ImportFailedError about missing dataset.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Create a dashboard export bundle (ZIP) that includes a
`dashboards/*.yaml` file whose
`metadata.chart_customization_config` contains a display control target with
a
`datasetUuid` (for example, `\"missing-dataset-uuid\"`) but remove the
corresponding
`datasets/*.yaml` (and/or `databases/*.yaml`) file from the bundle so that
no dataset
config with that UUID will be imported; this dashboard metadata path is
consumed by
`ImportDashboardsCommand._import` in
`superset/commands/dashboard/importers/v1/__init__.py:75-89`, which collects
dataset UUIDs
from display controls via `find_native_filter_datasets` in
`superset/commands/dashboard/importers/v1/utils.py:37-51`.
2. Run the dashboard import path, which executes
`ImportDashboardsCommand._import` in
`superset/commands/dashboard/importers/v1/__init__.py:70-218`; in the
dataset import loop
at `__init__.py:120-133`, `dataset_info` is populated only for datasets whose
`config["database_uuid"]` is present in `database_ids`, so the
removed/missing dataset
entry for `\"missing-dataset-uuid\"` is never added to `dataset_info`.
3. In the same `_import` method, the dashboard import loop at
`superset/commands/dashboard/importers/v1/__init__.py:169-175` iterates over
`dashboards/*.yaml` configs and calls `update_id_refs(config, chart_ids,
dataset_info)`,
passing the incomplete `dataset_info` map into `update_id_refs` in
`superset/commands/dashboard/importers/v1/utils.py:66-161`.
4. Inside `update_id_refs`, the new display-control remapping block at
`utils.py:149-158`
iterates `chart_customization_config` targets, executes `dataset_uuid =
target.pop("datasetUuid", None)` and then, because `dataset_uuid` is truthy,
evaluates
`target["datasetId"] = dataset_info[dataset_uuid]["datasource_id"]` (lines
156-158); since
`dataset_info` has no key for `\"missing-dataset-uuid\"`, this line raises a
`KeyError`,
which bubbles out of `update_id_refs` and `ImportDashboardsCommand._import`
as an
unhandled runtime exception instead of a controlled `ImportFailedError`,
causing the
dashboard ZIP import to crash whenever display controls reference a dataset
UUID that was
not successfully resolved into `dataset_info`.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset%2Fcommands%2Fdashboard%2Fimporters%2Fv1%2Futils.py%0A%2A%2ALine%3A%2A%2A%20156%3A158%0A%2A%2AComment%3A%2A%2A%0A%09%2ALogic%20Error%3A%20This%20new%20display-control%20remapping%20path%20can%20raise%20a%20%60KeyError%60%20when%20a%20dashboard%20references%20a%20%60datasetUuid%60%20that%20is%20not%20present%20in%20%60dataset_info%60%20%28for%20example%2C%20missing%20dataset%20files%2C%20failed%20dataset%20import%2C%20or%20partial%20bundles%29.%20That%20turns%20import%20into%20an%20unhandled%20runtime%20crash%20instead%20of%20a%20controlled%20import%20error.%20Guard%20the%20lookup%20before%20indexing%20%28or%20raise%20%60ImportFailedError%60%20with%20context%29%20so%20missing%20dataset%20mappings%20are%20handled%20deterministically.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%
20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset%2Fcommands%2Fdashboard%2Fimporters%2Fv1%2Futils.py%0A%2A%2ALine%3A%2A%2A%20156%3A158%0A%2A%2AComment%3A%2A%2A%0A%09%2ALogic%20Error%3A%20This%20new%20display-control%20remapping%20path%20can%20raise%20a%20%60KeyError%60%20when%20a%20dashboard%20references%20a%20%60datasetUuid%60%20that%20is%20not%20present%20in%20%60dataset_info%60%20%28for%20example%2C%20missing
%20dataset%20files%2C%20failed%20dataset%20import%2C%20or%20partial%20bundles%29.%20That%20turns%20import%20into%20an%20unhandled%20runtime%20crash%20instead%20of%20a%20controlled%20import%20error.%20Guard%20the%20lookup%20before%20indexing%20%28or%20raise%20%60ImportFailedError%60%20with%20context%29%20so%20missing%20dataset%20mappings%20are%20handled%20deterministically.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/commands/dashboard/importers/v1/utils.py
**Line:** 156:158
**Comment:**
*Logic Error: This new display-control remapping path can raise a
`KeyError` when a dashboard references a `datasetUuid` that is not present in
`dataset_info` (for example, missing dataset files, failed dataset import, or
partial bundles). That turns import into an unhandled runtime crash instead of
a controlled import error. Guard the lookup before indexing (or raise
`ImportFailedError` with context) so missing dataset mappings are handled
deterministically.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40008&comment_hash=dffc24fd222adee905d99544461f8b338e3ee918a5c6806427cd06de08c849b2&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40008&comment_hash=dffc24fd222adee905d99544461f8b338e3ee918a5c6806427cd06de08c849b2&reaction=dislike'>👎</a>
--
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]
