aminghadersohi commented on code in PR #40804:
URL: https://github.com/apache/superset/pull/40804#discussion_r3381316716
##########
superset/migrations/versions/2026-03-02_00-00_ce6bd21901ab_migrate_deckgl_and_mapbox.py:
##########
@@ -116,33 +136,56 @@ 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:
return False
- if "map_renderer" in params:
- return False
+ modified = False
+
+ mapbox_style = params.get("mapbox_style", "")
Review Comment:
`try_load_json()` can return valid non-object JSON (for example `[]`,
`"legacy"`, or `1`). A non-empty list/string reaches `.get()` here and raises
`AttributeError`, which `_migrate_deckgl_slices()` does not catch, aborting the
entire deployment migration after earlier batches may already have committed.
Validate `isinstance(params, dict)` before mutation (and add a
malformed/non-object params regression test).
##########
superset/migrations/versions/2026-03-02_00-00_ce6bd21901ab_migrate_deckgl_and_mapbox.py:
##########
@@ -116,33 +136,56 @@ 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:
return False
- if "map_renderer" in params:
- return False
+ modified = False
+
+ mapbox_style = params.get("mapbox_style", "")
+ if _is_mapbox_style(mapbox_style):
+ if "map_renderer" not in params:
+ params["map_renderer"] = "mapbox"
+ modified = True
+ else:
+ modified = _copy_legacy_maplibre_style(params)
- params["map_renderer"] = "mapbox"
+ if not modified:
+ return False
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 params:
return False
- params.pop("map_renderer", None)
+ modified = False
+ mapbox_style = params.get("mapbox_style")
+ if (
+ isinstance(mapbox_style, str)
+ and not _is_mapbox_style(mapbox_style)
+ and params.get("maplibre_style") == mapbox_style
+ ):
+ params.pop("maplibre_style", None)
+ modified = True
+
+ if "map_renderer" in params:
Review Comment:
The downgrade cannot tell which fields this migration added. A chart that
already had `map_renderer` is intentionally preserved by upgrade, but downgrade
deletes it; likewise, a pre-existing `maplibre_style` equal to `mapbox_style`
is removed. This makes upgrade→downgrade lossy. Record provenance/back up the
original deck.gl params, or only remove fields that can be proven to have been
introduced by this migration.
--
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]
