aminghadersohi commented on code in PR #40804:
URL: https://github.com/apache/superset/pull/40804#discussion_r3381779508
##########
superset/migrations/versions/2026-03-02_00-00_ce6bd21901ab_migrate_deckgl_and_mapbox.py:
##########
@@ -116,33 +143,58 @@ def upgrade_slice(cls, slc: Slice) -> None:
def _migrate_deckgl_slice(slc: Slice) -> bool:
- """Set map_renderer='mapbox' for all existing deck.gl slices.
+ """Preserve deck.gl renderer/style state after the MapLibre migration.
- This ensures full backwards compatibility: existing charts keep using the
- Mapbox renderer. Users can later switch to MapLibre in the chart controls.
- Only new charts will default to MapLibre.
+ True Mapbox styles get map_renderer='mapbox'. Non-Mapbox legacy
+ mapbox_style values are copied to maplibre_style so the MapLibre path keeps
+ rendering the saved style value.
Returns True if the slice was modified.
"""
params = try_load_json(slc.params)
- if not params:
+ if not isinstance(params, dict) or not params:
return False
- if "map_renderer" in params:
- return False
+ modified = False
+ added_fields: list[str] = []
+
+ mapbox_style = params.get("mapbox_style", "")
+ if _is_mapbox_style(mapbox_style):
+ if "map_renderer" not in params:
+ params["map_renderer"] = "mapbox"
+ added_fields.append("map_renderer")
+ modified = True
+ else:
+ modified = _copy_legacy_maplibre_style(params, added_fields)
- params["map_renderer"] = "mapbox"
+ if not modified:
+ return False
+ params[DECKGL_MIGRATION_ADDED_FIELDS] = added_fields
slc.params = json.dumps(params)
return True
def _downgrade_deckgl_slice(slc: Slice) -> bool:
"""Reverse _migrate_deckgl_slice. Returns True if the slice was
modified."""
params = try_load_json(slc.params)
- if not params or "map_renderer" not in params:
+ if not isinstance(params, dict) or not params:
+ return False
+
+ added_fields = params.get(DECKGL_MIGRATION_ADDED_FIELDS)
+ if not isinstance(added_fields, list):
return False
- params.pop("map_renderer", None)
+ modified = False
+ for field in added_fields:
+ if field in {"map_renderer", "maplibre_style"} and field in params:
+ params.pop(field, None)
+ modified = True
+
+ params.pop(DECKGL_MIGRATION_ADDED_FIELDS, None)
+ modified = True
+
+ if not modified:
+ return False
slc.params = json.dumps(params)
Review Comment:
`modified = True` on this line makes `if not modified: return False` on the
next line dead code — `modified` is unconditionally True at that point. The
behaviour is correct (sentinel cleanup always saves), but the guard is
misleading. Can simplify to `params.pop(DECKGL_MIGRATION_ADDED_FIELDS, None);
slc.params = json.dumps(params); return True`.
--
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]
