Re: [PR] fix(dashboard): handle missing chart metadata in dashboard export [superset]

["rusackas (via GitHub)" <gi...@apache.org>]

rusackas commented on code in PR #40575:
URL: https://github.com/apache/superset/pull/40575#discussion_r4028094719


##########
superset/commands/dashboard/importers/v1/utils.py:
##########
@@ -51,15 +51,20 @@ def find_native_filter_datasets(metadata: dict[str, Any]) -> set[str]:
 
 
 def build_uuid_to_id_map(position: dict[str, Any]) -> dict[str, int]:
-    return {
-        child["meta"]["uuid"]: child["meta"]["chartId"]
-        for child in position.values()
-        if (
-            isinstance(child, dict)
-            and child["type"] == "CHART"
-            and "uuid" in child["meta"]
-        )
-    }
+    result: dict[str, int] = {}
+    for child in position.values():
+        if not isinstance(child, dict):
+            continue
+        if child.get("type") != "CHART":
+            continue
+        meta = child.get("meta")
+        if not isinstance(meta, dict):
+            continue
+        uuid = meta.get("uuid")
+        chart_id = meta.get("chartId")
+        if uuid is not None and chart_id is not None:
+            result[uuid] = chart_id

Review Comment:
   Agreed, `uuid` could come through as a dict or list from a corrupt payload and `result[uuid]` would blow up on that. Might be worth an `isinstance(uuid, str)` check alongside the None checks here.



##########
superset/commands/dashboard/importers/v1/utils.py:
##########
@@ -120,6 +127,7 @@ def update_id_refs(  # pylint: disable=too-many-locals  # noqa: C901
         metadata["expanded_slices"] = {
             str(id_map[int(old_id)]): value
             for old_id, value in metadata["expanded_slices"].items()
+            if int(old_id) in id_map

Review Comment:
   Agreed, `int(old_id)` isnt guarded here so a non-numeric key would still crash the import instead of getting dropped. Same pattern shows up for `filter_scopes` and `default_filters` above too, might be worth a shared try/except.



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org