You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "richardfogaca (via GitHub)" <gi...@apache.org> on 2026/06/05 13:53:22 UTC
[PR] fix(maps): preserve OSM map renderer choices [superset]
richardfogaca opened a new pull request, #40804:
URL: https://github.com/apache/superset/pull/40804
### SUMMARY
Fixes map chart compatibility around the MapLibre migration:
- Preserves saved non-Mapbox Deck GL and legacy Mapbox-chart styles on the MapLibre path instead of reclassifying them as Mapbox.
- Keeps true saved `mapbox://` styles Mapbox-backed, so deployments without `MAPBOX_API_KEY` continue to show the existing missing-key signal rather than silently changing providers.
- Restores an explicit `Streets (OSM)` MapLibre-compatible style with OpenStreetMap tile attribution and shares the renderer/style helpers across Deck GL and point-cluster controls.
- Wires GeoJSON and Polygon renderer/style controls into the rendered map container so selected MapLibre/Mapbox styles affect output.
- Documents the migration behavior and no-key Mapbox affordance in `UPDATING.md`.
## What Changed
- Migration normalization: only true `mapbox://` styles choose the Mapbox renderer; non-Mapbox legacy style strings are copied to `maplibre_style` when that value is absent.
- Shared map utilities: OSM constants, raster tile style conversion, Mapbox key detection, and renderer option gating live in `@superset-ui/core` utilities and are reused by affected map plugins.
- Render propagation: GeoJSON and Polygon map form data is converted into provider-specific style and key props before rendering.
- Support notes: `UPDATING.md` documents non-Mapbox preservation, true Mapbox no-key behavior, and OSM attribution expectations.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A - no provider-visible media is attached in this draft. The affected UI behavior was exercised in a local Superset runtime: no-key Deck GL and point-cluster Explore controls showed `Streets (OSM)` as available with Mapbox disabled, saved true Mapbox charts showed the `MAPBOX_API_KEY` required message, and GeoJSON/Polygon charts rendered MapLibre/OSM maps with attribution.
### TESTING INSTRUCTIONS
Repeatable checks:
```bash
pytest tests/integration_tests/migrations/ce6bd21901ab_migrate_deckgl_and_mapbox__test.py -q
cd superset-frontend
npm run test -- packages/superset-ui-core/src/utils/mapStyles.test.ts plugins/preset-chart-deckgl/src/DeckGLContainer.test.tsx plugins/preset-chart-deckgl/src/utilities/Shared_DeckGL.test.ts plugins/preset-chart-deckgl/src/layers/Geojson/Geojson.test.tsx plugins/preset-chart-deckgl/src/layers/Polygon/Polygon.test.tsx plugins/plugin-chart-point-cluster-map/test/controlPanel.test.ts plugins/plugin-chart-point-cluster-map/test/MapLibre.test.tsx plugins/plugin-chart-point-cluster-map/test/transformProps.test.ts
```
## Validation
Live browser validation in a local Superset runtime with `MAPBOX_API_KEY` unset:
- Open Deck GL and point-cluster Explore controls; observe `Streets (OSM)` remains selectable and Mapbox is disabled for new selection.
- Open saved true Mapbox Deck GL and point-cluster charts; observe the existing `MAPBOX_API_KEY` required message is preserved.
- Open GeoJSON and Polygon charts configured for MapLibre/OSM; observe rendered map data and OpenStreetMap attribution, confirming renderer/style propagation reaches the shared map container.
## Reviewer Focus
- Review the migration normalization carefully: existing `map_renderer` and existing `maplibre_style` values should not be overwritten, and true `mapbox://` styles should remain Mapbox-backed.
- Check renderer option consistency across the Deck GL and point-cluster controls, especially no-key saved-state behavior.
- Check GeoJSON and Polygon provider/style propagation into the shared map container.
- The OSM style uses public OpenStreetMap tiles and must keep visible attribution.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [x] Changes UI
- [x] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [x] Migration is atomic, supports rollback & is backwards-compatible
- [x] Confirm DB migration upgrade and downgrade tested
- [x] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
Migration notes: this updates the existing MapLibre migration before an application release tag contains the upstream migration. The change only rewrites chart metadata JSON during the migration path, preserves existing renderer choices and existing `maplibre_style`, and keeps downgrade behavior scoped to removing the migration-added renderer marker. Runtime impact is bounded to the existing paginated chart-slice 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: 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
Re: [PR] fix(maps): preserve OSM styles and configurable renderer defaults [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #40804:
URL: https://github.com/apache/superset/pull/40804#issuecomment-4644013850
<!-- translation-regression-bot -->
## ⚠️ Translation Regression Detected
A source change in this PR renamed or reworded strings, invalidating existing translations (they are now `#, fuzzy`) in `ar`, `ca`, `cs`, `de`, `fi`, `fr`, `it`, `ja`, `ko`, `lv`, `mi`, `nl`, `pt`, `ru`, `sk`, `sl`, `th`, `uk`. Please resolve the affected `.po` files before merging.
_Note: intentionally **deleting** a translatable string is not a regression and is not flagged here — only translations invalidated by a renamed/reworded source string are._
| Language | Fuzzy before | Fuzzy after | New |
|----------|-------------:|------------:|----:|
| `ar` | 1136 | 1137 | +1 |
| `ca` | 815 | 816 | +1 |
| `cs` | 787 | 788 | +1 |
| `de` | 997 | 998 | +1 |
| `fi` | 4823 | 4824 | +1 |
| `fr` | 1014 | 1015 | +1 |
| `it` | 1667 | 1668 | +1 |
| `ja` | 322 | 323 | +1 |
| `ko` | 1521 | 1522 | +1 |
| `lv` | 295 | 296 | +1 |
| `mi` | 807 | 808 | +1 |
| `nl` | 1143 | 1144 | +1 |
| `pt` | 1839 | 1840 | +1 |
| `ru` | 293 | 294 | +1 |
| `sk` | 774 | 775 | +1 |
| `sl` | 963 | 964 | +1 |
| `th` | 4823 | 4824 | +1 |
| `uk` | 293 | 294 | +1 |
### How to fix
**1. Install dependencies** (if not already set up):
```bash
pip install -r superset/translations/requirements.txt
sudo apt-get install gettext # or: brew install gettext
```
**2. Re-extract strings and sync `.po` files:**
```bash
./scripts/translations/babel_update.sh
```
This rewrites `superset/translations/messages.pot` from the current source files and merges the changes into every `.po` file. Strings whose `msgid` changed will be marked `#, fuzzy`.
**3. Resolve the fuzzy entries** in the affected language files (`ar`, `ca`, `cs`, `de`, `fi`, `fr`, `it`, `ja`, `ko`, `lv`, `mi`, `nl`, `pt`, `ru`, `sk`, `sl`, `th`, `uk`):
```bash
grep -n '#, fuzzy' superset/translations/<lang>/LC_MESSAGES/messages.po
```
For each fuzzy entry, either rewrite the `msgstr` to match the new string and remove the `#, fuzzy` line, or clear the `msgstr` to `""` if you cannot provide a translation.
**4. Commit your changes to the `.po` files.**
--
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
Re: [PR] fix(maps): preserve OSM styles and configurable renderer defaults [superset]
Posted by "richardfogaca (via GitHub)" <gi...@apache.org>.
richardfogaca commented on PR #40804:
URL: https://github.com/apache/superset/pull/40804#issuecomment-4662415032
@aminghadersohi pushed the follow-up now: `d336285df5` addresses the remaining type-hygiene cleanup, the downgrade sentinel dead branch, and the point-cluster OSM translation wrapper. Codecov has restarted on the new head, so that gate is still pending.
--
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
Re: [PR] fix(maps): preserve OSM styles and configurable renderer defaults [superset]
Posted by "richardfogaca (via GitHub)" <gi...@apache.org>.
richardfogaca commented on PR #40804:
URL: https://github.com/apache/superset/pull/40804#issuecomment-4661471531
@aminghadersohi quick update before the next push: I addressed the remaining type-hygiene items in `Shared_DeckGL.tsx` and point-cluster `controlPanel.ts`, simplified the downgrade sentinel cleanup dead branch, and wrapped the remaining point-cluster `Streets (OSM)` label in `t(...)`. I also verified the non-object migration params guard/tests are already present at the current head. Codecov is still the remaining separate gate to work through.
--
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
Re: [PR] fix(maps): preserve OSM styles and configurable renderer defaults [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #40804:
URL: https://github.com/apache/superset/pull/40804#issuecomment-4644018745
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/40804?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 55.46%. Comparing base ([`5da63d7`](https://app.codecov.io/gh/apache/superset/commit/5da63d716bef071a2a71e45fae0d134af0b513af?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`bdc13a8`](https://app.codecov.io/gh/apache/superset/commit/bdc13a8ebaac3a3ef49d8241ac1c3b1de130a99c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
> :exclamation: There is a different number of reports uploaded between BASE (5da63d7) and HEAD (bdc13a8). Click for more details.
>
> <details><summary>HEAD has 81 uploads less than BASE</summary>
>
>| Flag | BASE (5da63d7) | HEAD (bdc13a8) |
>|------|------|------|
>|python|71|1|
>|presto|12|1|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #40804 +/- ##
==========================================
- Coverage 64.14% 55.46% -8.69%
==========================================
Files 2650 2650
Lines 143455 143426 -29
Branches 33097 33085 -12
==========================================
- Hits 92019 79551 -12468
- Misses 49827 63160 +13333
+ Partials 1609 715 -894
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/40804/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/40804/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/40804/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/40804/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/40804/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `41.12% <100.00%> (+<0.01%)` | :arrow_up: |
| [python](https://app.codecov.io/gh/apache/superset/pull/40804/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `41.12% <100.00%> (-18.70%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/40804/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/40804/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/40804?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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
Re: [PR] fix(maps): preserve OSM styles and configurable renderer defaults [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40804:
URL: https://github.com/apache/superset/pull/40804#issuecomment-4663877834
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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
Re: [PR] fix(maps): preserve OSM styles and configurable renderer defaults [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
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: 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
Re: [PR] fix(maps): preserve OSM styles and configurable renderer defaults [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on code in PR #40804:
URL: https://github.com/apache/superset/pull/40804#discussion_r3382841242
##########
superset-frontend/plugins/preset-chart-deckgl/src/utilities/Shared_DeckGL.tsx:
##########
@@ -702,7 +775,7 @@ export const deckGLBreakpointMetric: CustomControlItem = {
// mapStateToProps: (state: ControlPanelState) => ({
// datasource: state.datasource,
// }),
- visibility: ({ controls }: { controls: any }) =>
+ visibility: ({ controls }: MapStyleVisibilityProps) =>
Review Comment:
`MapStyleVisibilityProps` makes `controls` optional (`controls?: {...}`), but `isColorSchemeTypeVisible` at line 779 expects `ControlStateMapping` (non-optional) — causes TS2345 in CI. Add a guard: `if (!controls) return false;` before the call, or remove the `?` from `controls` in `MapStyleVisibilityProps`.
##########
superset-frontend/plugins/preset-chart-deckgl/src/utilities/Shared_DeckGL.tsx:
##########
@@ -555,7 +628,7 @@ export const tooltipContents = {
),
ghostButtonText: t('Drop columns/metrics here or click'),
disabledTabs: new Set(['saved', 'sqlExpression']),
- mapStateToProps: (state: any) => {
+ mapStateToProps: (state: ControlPanelState) => {
Review Comment:
Changing `state: any` to `state: ControlPanelState` exposes a type error at line 640: `datasource?.metrics` fails because `ControlPanelState.datasource` is `Dataset | QueryResponse` and `QueryResponse` has no `metrics` property — breaks `lint-frontend` and `validate-frontend` in CI. Fix with a type guard: `savedMetrics: datasource && 'metrics' in datasource ? datasource.metrics : []`.
--
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
Re: [PR] fix(maps): preserve OSM styles and configurable renderer defaults [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40804:
URL: https://github.com/apache/superset/pull/40804#issuecomment-4661820112
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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
Re: [PR] fix(maps): preserve OSM styles and configurable renderer defaults [superset]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #40804:
URL: https://github.com/apache/superset/pull/40804#discussion_r3369799507
##########
superset/migrations/versions/2026-03-02_00-00_ce6bd21901ab_migrate_deckgl_and_mapbox.py:
##########
@@ -61,6 +61,22 @@
]
+def _is_mapbox_style(style: Any) -> bool:
+ return isinstance(style, str) and style.startswith("mapbox://")
+
+
+def _copy_legacy_maplibre_style(data: dict[str, Any]) -> bool:
+ mapbox_style = data.get("mapbox_style")
+ if (
+ isinstance(mapbox_style, str)
+ and not _is_mapbox_style(mapbox_style)
+ and "maplibre_style" not in data
+ ):
+ data["maplibre_style"] = mapbox_style
+ return True
Review Comment:
When copying a legacy non-`mapbox://` `mapbox_style` into `maplibre_style`, the migration does not persist `map_renderer='maplibre'`. In Explore, missing control values are filled from the control `default`, so deployments with `DEFAULT_MAP_RENDERER=mapbox` + a Mapbox key can end up opening these migrated charts with Mapbox selected, undermining the intent to keep non-Mapbox/OSM styles on the MapLibre path and potentially breaking raster tile templates.
Consider also setting `map_renderer` to `'maplibre'` when (and only when) you copy a legacy style and `map_renderer` is absent, so existing charts remain pinned to MapLibre regardless of deployment defaults.
--
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
Re: [PR] fix(maps): preserve OSM styles and configurable renderer defaults [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40804:
URL: https://github.com/apache/superset/pull/40804#issuecomment-4653425233
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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
Re: [PR] fix(maps): preserve OSM styles and configurable renderer defaults [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on code in PR #40804:
URL: https://github.com/apache/superset/pull/40804#discussion_r3383016015
##########
superset-frontend/packages/superset-ui-core/src/utils/mapStyles.ts:
##########
@@ -0,0 +1,238 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+export type MapProvider = 'maplibre' | 'mapbox';
+
+export type MapRendererOption = {
+ value: MapProvider;
+ disabled?: boolean;
+};
+
+export type MapProviderMapStyle = {
+ mapProvider?: unknown;
+ maplibreStyle?: unknown;
+ mapboxStyle?: unknown;
+ legacyMapStyle?: unknown;
+};
+
+export type SelectedMapProviderMapStyle = {
+ mapProvider: MapProvider;
+ mapStyle?: string;
+};
+
+export type RasterTileMapStyle = {
+ version: 8;
+ sources: {
+ [sourceId: string]: {
+ type: 'raster';
+ tiles: string[];
+ tileSize: 256;
+ attribution?: string;
+ };
+ };
+ layers: [
+ {
+ id: string;
+ type: 'raster';
+ source: string;
+ minzoom: 0;
+ maxzoom: 22;
+ },
+ ];
+};
+
+export type ResolvedMapStyle = string | RasterTileMapStyle;
+
+export const OSM_TILE_STYLE_URL =
+ 'https://tile.openstreetmap.org/{z}/{x}/{y}.png';
+export const OSM_TILE_ATTRIBUTION = '© OpenStreetMap contributors';
+
+export const MAPLIBRE_RENDERER_OPTION: MapRendererOption = {
+ value: 'maplibre',
+};
+export const MAPBOX_RENDERER_OPTION: MapRendererOption = {
+ value: 'mapbox',
+};
+export const DISABLED_MAPBOX_RENDERER_OPTION: MapRendererOption = {
+ ...MAPBOX_RENDERER_OPTION,
+ disabled: true,
+};
+
+const TILE_PROTOCOL = 'tile://';
+const RASTER_SOURCE_ID = 'osm-raster-tiles';
+const RASTER_LAYER_ID = 'osm-raster-layer';
+
+type BootstrapData = {
+ common?: {
+ conf?: {
+ DEFAULT_MAP_RENDERER?: unknown;
+ MAPBOX_API_KEY?: unknown;
+ };
+ };
+};
+
+export function getBootstrapDataFromDocument(): unknown {
+ if (typeof document === 'undefined') {
+ return undefined;
+ }
+
+ try {
+ const appContainer = document.getElementById('app');
+ const dataBootstrap = appContainer?.getAttribute('data-bootstrap');
+ return dataBootstrap ? JSON.parse(dataBootstrap) : undefined;
+ } catch {
+ return undefined;
+ }
+}
+
+export function getMapboxApiKeyFromBootstrap(
+ bootstrapData: unknown = getBootstrapDataFromDocument(),
+): string {
+ const mapboxApiKey = (bootstrapData as BootstrapData | undefined)?.common
+ ?.conf?.MAPBOX_API_KEY;
+ return typeof mapboxApiKey === 'string' ? mapboxApiKey.trim() : '';
+}
+
+export function hasMapboxApiKey(
+ bootstrapData: unknown = getBootstrapDataFromDocument(),
+): boolean {
+ return getMapboxApiKeyFromBootstrap(bootstrapData).trim().length > 0;
+}
+
+export function getDefaultMapRenderer(
+ bootstrapData: unknown = getBootstrapDataFromDocument(),
+): MapProvider {
+ const conf = (bootstrapData as BootstrapData | undefined)?.common?.conf;
+ const defaultRenderer = conf?.DEFAULT_MAP_RENDERER;
+
+ if (defaultRenderer === 'mapbox' && hasMapboxApiKey(bootstrapData)) {
+ return 'mapbox';
+ }
+
+ return 'maplibre';
+}
+
+export function getMapRendererOptions({
+ hasMapboxKey,
+ currentValue,
+}: {
+ hasMapboxKey: boolean;
+ currentValue?: MapProvider;
+}): MapRendererOption[] {
+ if (!hasMapboxKey && currentValue !== 'mapbox') {
+ return [MAPLIBRE_RENDERER_OPTION];
+ }
+
+ return [
+ MAPLIBRE_RENDERER_OPTION,
+ hasMapboxKey ? MAPBOX_RENDERER_OPTION : DISABLED_MAPBOX_RENDERER_OPTION,
+ ];
+}
+
+function getNonEmptyString(value: unknown): string | undefined {
+ return typeof value === 'string' && value.trim().length > 0
+ ? value
+ : undefined;
+}
+
+function isMapboxStyle(value: unknown): boolean {
+ return getNonEmptyString(value)?.startsWith('mapbox://') ?? false;
+}
+
+export function getMapProviderMapStyle({
+ mapProvider,
+ maplibreStyle,
+ mapboxStyle,
+ legacyMapStyle,
+}: MapProviderMapStyle): SelectedMapProviderMapStyle {
+ const selectedMapProvider: MapProvider =
+ mapProvider === 'mapbox' ? 'mapbox' : 'maplibre';
+ const maplibreStyleValue = getNonEmptyString(maplibreStyle);
+ const mapboxStyleValue = getNonEmptyString(mapboxStyle);
+ const legacyMapStyleValue = getNonEmptyString(legacyMapStyle);
+
+ if (selectedMapProvider === 'mapbox') {
+ return {
+ mapProvider: selectedMapProvider,
+ mapStyle: mapboxStyleValue ?? legacyMapStyleValue,
+ };
+ }
+
+ return {
+ mapProvider: selectedMapProvider,
+ mapStyle:
+ maplibreStyleValue ??
+ (isMapboxStyle(mapboxStyleValue) ? undefined : mapboxStyleValue) ??
+ legacyMapStyleValue,
+ };
+}
+
+function unwrapTileProtocol(value: string): string {
+ return value.startsWith(TILE_PROTOCOL)
+ ? value.slice(TILE_PROTOCOL.length)
+ : value;
+}
+
+export function isRasterTileTemplate(value: unknown): value is string {
+ if (typeof value !== 'string') {
+ return false;
+ }
+ const tileUrl = unwrapTileProtocol(value);
+ return ['{z}', '{x}', '{y}'].every(templateParam =>
+ tileUrl.includes(templateParam),
+ );
+}
+
+export function buildRasterTileMapStyle(value: string): RasterTileMapStyle {
+ const tileUrl = unwrapTileProtocol(value);
+ const attribution =
+ tileUrl === OSM_TILE_STYLE_URL ? { attribution: OSM_TILE_ATTRIBUTION } : {};
Review Comment:
Exact `=== OSM_TILE_STYLE_URL` check misses documented OSM subdomain variants (e.g. `https://c.tile.openstreetmap.org/{z}/{x}/{y}.png`), so those URLs render without the required OSM attribution. Detect by hostname instead:
```suggestion
tileUrl.includes('openstreetmap.org/') ? { attribution: OSM_TILE_ATTRIBUTION } : {};
```
--
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
Re: [PR] fix(maps): preserve OSM map renderer choices [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #40804:
URL: https://github.com/apache/superset/pull/40804#issuecomment-4632314552
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 29a22cee9def33c12e1b515b4fb20420d20b777e |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a22d4d26c9bc50008590328 |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-40804--superset-docs-preview.netlify.app](https://deploy-preview-40804--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcpBCoAgEADAv-y5BbOU7Ac9Q1fBQ-ayShDR3_M2h3mBpRbusANMEMRflIeFMs6zQ5bUktwJaytYPCNlL72NGhOf9TniyNZrHdeoLblARim1GacWvcH3Aw) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-40804--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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
Re: [PR] fix(maps): preserve OSM map renderer choices [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40804:
URL: https://github.com/apache/superset/pull/40804#issuecomment-4633876962
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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
Re: [PR] fix(maps): preserve OSM map renderer choices [superset]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #40804:
URL: https://github.com/apache/superset/pull/40804#discussion_r3363320959
##########
superset-frontend/packages/superset-ui-core/src/utils/mapStyles.ts:
##########
@@ -0,0 +1,180 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+export type MapProvider = 'maplibre' | 'mapbox';
+
Review Comment:
This module introduces new user-facing labels (renderer options / OSM style choice) but doesn’t use the standard `t(...)` translation helper, so these strings won’t be localizable in Explore controls.
##########
superset-frontend/packages/superset-ui-core/src/utils/mapStyles.ts:
##########
@@ -0,0 +1,180 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+export type MapProvider = 'maplibre' | 'mapbox';
+
+export type MapStyleChoice = {
+ value: string;
+ label: string;
+ attribution?: string;
+};
+
+export type MapRendererOption = {
+ value: MapProvider;
+ label: string;
+ disabled?: boolean;
+};
+
+export type RasterTileMapStyle = {
+ version: 8;
+ sources: {
+ [sourceId: string]: {
+ type: 'raster';
+ tiles: string[];
+ tileSize: 256;
+ attribution: string;
+ };
+ };
+ layers: [
+ {
+ id: string;
+ type: 'raster';
+ source: string;
+ minzoom: 0;
+ maxzoom: 22;
+ },
+ ];
+};
+
+export type ResolvedMapStyle = string | RasterTileMapStyle;
+
+export const OSM_TILE_STYLE_URL =
+ 'https://tile.openstreetmap.org/{z}/{x}/{y}.png';
+export const OSM_TILE_ATTRIBUTION = '© OpenStreetMap contributors';
+export const OSM_TILE_STYLE_CHOICE: MapStyleChoice = {
+ value: OSM_TILE_STYLE_URL,
+ label: 'Streets (OSM)',
+ attribution: OSM_TILE_ATTRIBUTION,
+};
Review Comment:
`OSM_TILE_STYLE_CHOICE.label` is displayed in UI controls; it should be wrapped in `t(...)` so it can be translated like other control labels.
##########
superset-frontend/packages/superset-ui-core/src/utils/mapStyles.ts:
##########
@@ -0,0 +1,180 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+export type MapProvider = 'maplibre' | 'mapbox';
+
+export type MapStyleChoice = {
+ value: string;
+ label: string;
+ attribution?: string;
+};
+
+export type MapRendererOption = {
+ value: MapProvider;
+ label: string;
+ disabled?: boolean;
+};
+
+export type RasterTileMapStyle = {
+ version: 8;
+ sources: {
+ [sourceId: string]: {
+ type: 'raster';
+ tiles: string[];
+ tileSize: 256;
+ attribution: string;
+ };
+ };
+ layers: [
+ {
+ id: string;
+ type: 'raster';
+ source: string;
+ minzoom: 0;
+ maxzoom: 22;
+ },
+ ];
+};
+
+export type ResolvedMapStyle = string | RasterTileMapStyle;
+
+export const OSM_TILE_STYLE_URL =
+ 'https://tile.openstreetmap.org/{z}/{x}/{y}.png';
+export const OSM_TILE_ATTRIBUTION = '© OpenStreetMap contributors';
+export const OSM_TILE_STYLE_CHOICE: MapStyleChoice = {
+ value: OSM_TILE_STYLE_URL,
+ label: 'Streets (OSM)',
+ attribution: OSM_TILE_ATTRIBUTION,
+};
+
+export const MAPLIBRE_RENDERER_OPTION: MapRendererOption = {
+ value: 'maplibre',
+ label: 'MapLibre (open-source)',
+};
+export const MAPBOX_RENDERER_OPTION: MapRendererOption = {
+ value: 'mapbox',
+ label: 'Mapbox (API key required)',
+};
+export const DISABLED_MAPBOX_RENDERER_OPTION: MapRendererOption = {
+ ...MAPBOX_RENDERER_OPTION,
+ label: 'Mapbox (MAPBOX_API_KEY required)',
+ disabled: true,
+};
+
+const TILE_PROTOCOL = 'tile://';
+const RASTER_SOURCE_ID = 'osm-raster-tiles';
+const RASTER_LAYER_ID = 'osm-raster-layer';
+
+type BootstrapData = {
+ common?: {
+ conf?: {
+ MAPBOX_API_KEY?: unknown;
+ };
+ };
+};
+
+export function getBootstrapDataFromDocument(): unknown {
+ if (typeof document === 'undefined') {
+ return undefined;
+ }
+
+ try {
+ const appContainer = document.getElementById('app');
+ const dataBootstrap = appContainer?.getAttribute('data-bootstrap');
+ return dataBootstrap ? JSON.parse(dataBootstrap) : undefined;
+ } catch {
+ return undefined;
+ }
+}
+
+export function getMapboxApiKeyFromBootstrap(
+ bootstrapData: unknown = getBootstrapDataFromDocument(),
+): string {
+ const mapboxApiKey = (bootstrapData as BootstrapData | undefined)?.common
+ ?.conf?.MAPBOX_API_KEY;
+ return typeof mapboxApiKey === 'string' ? mapboxApiKey : '';
+}
+
+export function hasMapboxApiKey(
+ bootstrapData: unknown = getBootstrapDataFromDocument(),
+): boolean {
+ return getMapboxApiKeyFromBootstrap(bootstrapData).trim().length > 0;
+}
+
+export function getMapRendererOptions({
+ hasMapboxKey,
+}: {
+ hasMapboxKey: boolean;
+ currentValue?: MapProvider;
+}): MapRendererOption[] {
+ return [
+ MAPLIBRE_RENDERER_OPTION,
+ hasMapboxKey ? MAPBOX_RENDERER_OPTION : DISABLED_MAPBOX_RENDERER_OPTION,
+ ];
+}
+
+function unwrapTileProtocol(value: string): string {
+ return value.startsWith(TILE_PROTOCOL)
+ ? value.slice(TILE_PROTOCOL.length)
+ : value;
+}
+
+export function isRasterTileTemplate(value: unknown): value is string {
+ if (typeof value !== 'string') {
+ return false;
+ }
+ const tileUrl = unwrapTileProtocol(value);
+ return ['{z}', '{x}', '{y}'].every(templateParam =>
+ tileUrl.includes(templateParam),
+ );
+}
+
+export function buildRasterTileMapStyle(value: string): RasterTileMapStyle {
+ return {
+ version: 8,
+ sources: {
+ [RASTER_SOURCE_ID]: {
+ type: 'raster',
+ tiles: [unwrapTileProtocol(value)],
+ tileSize: 256,
+ attribution: OSM_TILE_ATTRIBUTION,
+ },
+ },
+ layers: [
+ {
+ id: RASTER_LAYER_ID,
+ type: 'raster',
+ source: RASTER_SOURCE_ID,
+ minzoom: 0,
+ maxzoom: 22,
+ },
+ ],
+ };
+}
Review Comment:
`buildRasterTileMapStyle()` assigns OpenStreetMap attribution to *any* raster tile template (anything containing {z}/{x}/{y}). This is incorrect for non-OSM tile servers and can misattribute third-party map data.
##########
superset-frontend/packages/superset-ui-core/src/utils/mapStyles.ts:
##########
@@ -0,0 +1,180 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+export type MapProvider = 'maplibre' | 'mapbox';
+
+export type MapStyleChoice = {
+ value: string;
+ label: string;
+ attribution?: string;
+};
+
+export type MapRendererOption = {
+ value: MapProvider;
+ label: string;
+ disabled?: boolean;
+};
+
+export type RasterTileMapStyle = {
+ version: 8;
+ sources: {
+ [sourceId: string]: {
+ type: 'raster';
+ tiles: string[];
+ tileSize: 256;
+ attribution: string;
+ };
+ };
+ layers: [
+ {
+ id: string;
+ type: 'raster';
+ source: string;
+ minzoom: 0;
+ maxzoom: 22;
+ },
+ ];
+};
+
+export type ResolvedMapStyle = string | RasterTileMapStyle;
+
+export const OSM_TILE_STYLE_URL =
+ 'https://tile.openstreetmap.org/{z}/{x}/{y}.png';
+export const OSM_TILE_ATTRIBUTION = '© OpenStreetMap contributors';
+export const OSM_TILE_STYLE_CHOICE: MapStyleChoice = {
+ value: OSM_TILE_STYLE_URL,
+ label: 'Streets (OSM)',
+ attribution: OSM_TILE_ATTRIBUTION,
+};
+
+export const MAPLIBRE_RENDERER_OPTION: MapRendererOption = {
+ value: 'maplibre',
+ label: 'MapLibre (open-source)',
+};
+export const MAPBOX_RENDERER_OPTION: MapRendererOption = {
+ value: 'mapbox',
+ label: 'Mapbox (API key required)',
+};
Review Comment:
`MAPBOX_RENDERER_OPTION.label` is a user-facing control label; wrap it in `t(...)` so it remains translatable.
##########
superset-frontend/packages/superset-ui-core/src/utils/mapStyles.ts:
##########
@@ -0,0 +1,180 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+export type MapProvider = 'maplibre' | 'mapbox';
+
+export type MapStyleChoice = {
+ value: string;
+ label: string;
+ attribution?: string;
+};
+
+export type MapRendererOption = {
+ value: MapProvider;
+ label: string;
+ disabled?: boolean;
+};
+
+export type RasterTileMapStyle = {
+ version: 8;
+ sources: {
+ [sourceId: string]: {
+ type: 'raster';
+ tiles: string[];
+ tileSize: 256;
+ attribution: string;
+ };
+ };
+ layers: [
+ {
+ id: string;
+ type: 'raster';
+ source: string;
+ minzoom: 0;
+ maxzoom: 22;
+ },
+ ];
+};
+
+export type ResolvedMapStyle = string | RasterTileMapStyle;
+
+export const OSM_TILE_STYLE_URL =
+ 'https://tile.openstreetmap.org/{z}/{x}/{y}.png';
+export const OSM_TILE_ATTRIBUTION = '© OpenStreetMap contributors';
+export const OSM_TILE_STYLE_CHOICE: MapStyleChoice = {
+ value: OSM_TILE_STYLE_URL,
+ label: 'Streets (OSM)',
+ attribution: OSM_TILE_ATTRIBUTION,
+};
+
+export const MAPLIBRE_RENDERER_OPTION: MapRendererOption = {
+ value: 'maplibre',
+ label: 'MapLibre (open-source)',
+};
Review Comment:
`MAPLIBRE_RENDERER_OPTION.label` is a user-facing control label; wrap it in `t(...)` to keep renderer choices localizable.
##########
superset-frontend/packages/superset-ui-core/src/utils/mapStyles.ts:
##########
@@ -0,0 +1,180 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+export type MapProvider = 'maplibre' | 'mapbox';
+
+export type MapStyleChoice = {
+ value: string;
+ label: string;
+ attribution?: string;
+};
+
+export type MapRendererOption = {
+ value: MapProvider;
+ label: string;
+ disabled?: boolean;
+};
+
+export type RasterTileMapStyle = {
+ version: 8;
+ sources: {
+ [sourceId: string]: {
+ type: 'raster';
+ tiles: string[];
+ tileSize: 256;
+ attribution: string;
+ };
+ };
+ layers: [
+ {
+ id: string;
+ type: 'raster';
+ source: string;
+ minzoom: 0;
+ maxzoom: 22;
+ },
+ ];
+};
+
+export type ResolvedMapStyle = string | RasterTileMapStyle;
+
+export const OSM_TILE_STYLE_URL =
+ 'https://tile.openstreetmap.org/{z}/{x}/{y}.png';
+export const OSM_TILE_ATTRIBUTION = '© OpenStreetMap contributors';
+export const OSM_TILE_STYLE_CHOICE: MapStyleChoice = {
+ value: OSM_TILE_STYLE_URL,
+ label: 'Streets (OSM)',
+ attribution: OSM_TILE_ATTRIBUTION,
+};
+
+export const MAPLIBRE_RENDERER_OPTION: MapRendererOption = {
+ value: 'maplibre',
+ label: 'MapLibre (open-source)',
+};
+export const MAPBOX_RENDERER_OPTION: MapRendererOption = {
+ value: 'mapbox',
+ label: 'Mapbox (API key required)',
+};
+export const DISABLED_MAPBOX_RENDERER_OPTION: MapRendererOption = {
+ ...MAPBOX_RENDERER_OPTION,
+ label: 'Mapbox (MAPBOX_API_KEY required)',
+ disabled: true,
+};
Review Comment:
`DISABLED_MAPBOX_RENDERER_OPTION.label` is shown in Explore when Mapbox is disabled; it should be translated via `t(...)` like the other renderer labels.
--
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
Re: [PR] fix(maps): preserve OSM styles and configurable renderer defaults [superset]
Posted by "richardfogaca (via GitHub)" <gi...@apache.org>.
richardfogaca merged PR #40804:
URL: https://github.com/apache/superset/pull/40804
--
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
Re: [PR] fix(maps): preserve OSM styles and configurable renderer defaults [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40804:
URL: https://github.com/apache/superset/pull/40804#issuecomment-4643974552
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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
Re: [PR] fix(maps): preserve OSM styles and configurable renderer defaults [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on code in PR #40804:
URL: https://github.com/apache/superset/pull/40804#discussion_r3376253283
##########
superset-frontend/plugins/preset-chart-deckgl/src/utilities/Shared_DeckGL.tsx:
##########
@@ -40,15 +48,23 @@ import {
isColorSchemeTypeVisible,
} from './utils';
import { TooltipTemplateControl } from './TooltipTemplateControl';
+import { hasMapboxApiKey } from '../utils/mapbox';
const categoricalSchemeRegistry = getCategoricalSchemeRegistry();
const sequentialSchemeRegistry = getSequentialSchemeRegistry();
export const DEFAULT_DECKGL_COLOR = { r: 158, g: 158, b: 158, a: 1 };
-let deckglTiles: string[][];
+type DeckGLTileChoice = [string, string];
+type MapStyleVisibilityProps = {
+ controls?: {
+ map_renderer?: { value?: unknown };
+ };
+};
+
+let deckglTiles: DeckGLTileChoice[];
Review Comment:
`deckglTiles` is a module-level mutable singleton: once `getDeckGLTiles()` is called for the first time it is frozen for the module lifetime — `jest.resetModules()` on *other* modules does not clear it. Tests that load this module first (even transitively) silently fix the tile choices for all later tests in the same worker. The new tests in `Shared_DeckGL.test.ts` carefully work around this by dynamically importing after `jest.resetModules()`, but the pattern will bite the next contributor who adds a test. Consider exporting a `__resetDeckGLTilesForTesting` guard or removing the cache entirely (bootstrap reads are cheap).
##########
superset-frontend/plugins/preset-chart-deckgl/src/utilities/Shared_DeckGL.tsx:
##########
@@ -456,15 +507,25 @@ export const mapProvider = {
label: t('Map Renderer'),
clearable: false,
renderTrigger: true,
- choices: [
- ['maplibre', t('MapLibre (open-source)')],
- ['mapbox', t('Mapbox (API key required)')],
- ],
- default: 'maplibre',
+ options: getLabeledMapRendererOptions({
+ hasMapboxKey: hasMapboxApiKey(),
+ }),
+ default: getDefaultMapRenderer(),
Review Comment:
`getDefaultMapRenderer()` reads `document.getElementById('app')` at module import time and bakes the result as the permanent `default`. In jest/jsdom, modules may be imported before the bootstrap element is present, making this silently return `'maplibre'` regardless of `DEFAULT_MAP_RENDERER` config. The tests in `Shared_DeckGL.test.ts` already demonstrate this by requiring `jest.resetModules()` + dynamic `import()` just to test `config.default` correctly. Consider moving the default resolution into `mapStateToProps` so it is evaluated at render time.
--
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
Re: [PR] fix(maps): preserve OSM styles and configurable renderer defaults [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #40804:
URL: https://github.com/apache/superset/pull/40804#discussion_r3376379574
##########
superset-frontend/plugins/plugin-chart-point-cluster-map/src/utils/mapControls.ts:
##########
@@ -0,0 +1,60 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { t } from '@apache-superset/core/translation';
+import {
+ getMapRendererOptions,
+ OSM_TILE_STYLE_URL,
+ type MapRendererOption,
+ type MapProvider,
+} from '@superset-ui/core/utils/mapStyles';
+import { hasMapboxApiKey } from './mapbox';
+
+export const POINT_CLUSTER_MAPLIBRE_STYLE_CHOICES = [
+ ['https://tiles.openfreemap.org/styles/liberty', t('Liberty (OpenFreeMap)')],
+ [
+ 'https://basemaps.cartocdn.com/gl/positron-gl-style/style.json',
+ t('Light (Carto)'),
+ ],
+ [
+ 'https://basemaps.cartocdn.com/gl/dark-matter-gl-style/style.json',
+ t('Dark (Carto)'),
+ ],
+ [
+ 'https://basemaps.cartocdn.com/gl/voyager-gl-style/style.json',
+ t('Streets (Carto)'),
+ ],
+ [OSM_TILE_STYLE_URL, 'Streets (OSM)'],
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing translation wrapper on label</b></div>
<div id="fix">
The 'Streets (OSM)' label at line 43 is not wrapped with `t()`, while all other map style labels in the same array (lines 30, 33, 37, 41) use `t()` for internationalization. Per BITO.md rule [6516], all user-facing text must use translation functions to ensure multi-language support.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- a/superset-frontend/plugins/plugin-chart-point-cluster-map/src/utils/mapControls.ts
+++ b/superset-frontend/plugins/plugin-chart-point-cluster-map/src/utils/mapControls.ts
@@ -40,6 +40,6 @@ export const POINT_CLUSTER_MAPLIBRE_STYLE_CHOICES = [
t('Streets (Carto)'),
],
- [OSM_TILE_STYLE_URL, 'Streets (OSM)'],
+ [OSM_TILE_STYLE_URL, t('Streets (OSM)')],
];
```
</div>
</details>
</div>
<small><i>Code Review Run #b84e1f</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: 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
Re: [PR] fix(maps): preserve OSM map renderer choices [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40804:
URL: https://github.com/apache/superset/pull/40804#discussion_r3364217342
##########
superset-frontend/plugins/preset-chart-deckgl/src/utilities/Shared_DeckGL.tsx:
##########
@@ -73,13 +88,27 @@ export const DEFAULT_MAPBOX_TILES = [
['mapbox://styles/mapbox/outdoors-v9', 'Outdoors (Mapbox)'],
];
+const isDeckGLTileChoices = (value: unknown): value is DeckGLTileChoice[] =>
+ Array.isArray(value) &&
+ value.every(
+ choice =>
+ Array.isArray(choice) &&
+ choice.length >= 2 &&
+ typeof choice[0] === 'string' &&
+ typeof choice[1] === 'string',
+ );
Review Comment:
**Suggestion:** The tile-choice validator accepts an empty array as valid, so a deployment with `DECKGL_BASE_MAP=[]` will set `deckglTiles` to `[]` and later crash when `maplibreStyle` reads `getDeckGLTiles()[0][0]`. Tighten the validator to require at least one entry (and ideally exactly two string elements per choice) before accepting overrides, otherwise fall back to `DEFAULT_DECKGL_TILES`. [incorrect condition logic]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ DeckGL Explore controls crash when DECKGL_BASE_MAP is empty.
- ⚠️ All DeckGL map layers become unusable under misconfiguration.
- ⚠️ Admins cannot rely on empty list as override.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure Superset with an empty DeckGL base map list by setting `DECKGL_BASE_MAP = []`
in `superset_config.py` as described in
`docs/admin_docs/configuration/map-tiles.mdx:13-34`. When the server builds bootstrap data
in `superset/views/base.py:16`, it copies this configuration into the `"deckgl_tiles"`
field of the `bootstrap_data` dict.
2. Start the Superset web server so that the frontend receives `deckgl_tiles: []` in the
bootstrap payload embedded in the HTML, which is then read by
`getBootstrapDataFromDocument()` inside
`superset-frontend/plugins/preset-chart-deckgl/src/utilities/Shared_DeckGL.tsx` (see the
`getDeckGLTiles` function around lines 22–32 in the Read output).
3. In the browser, open Explore for any DeckGL-based chart such as the Hex layer
(`superset-frontend/plugins/preset-chart-deckgl/src/layers/Hex/controlPanel.ts:25-47`),
which imports `maplibreStyle` from `../../utilities/Shared_DeckGL` and includes it in the
`"Map"` control section.
4. When the control panel renders, `maplibreStyle.config` in `Shared_DeckGL.tsx` (shown in
the Read output lines 21–31) evaluates `choices: getDeckGLTiles()` and `default:
getDeckGLTiles()[0][0]`. Because `isDeckGLTileChoices` (PR lines 91–99) treats `[]` as
valid, `getDeckGLTiles()` returns `[]`, so `getDeckGLTiles()[0]` is `undefined` and `[0]`
access throws a runtime error in the browser, breaking the Explore UI for DeckGL charts
under this configuration.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=35af6f5458ff4801b4a3374f8ded60ff&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=35af6f5458ff4801b4a3374f8ded60ff&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(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-frontend/plugins/preset-chart-deckgl/src/utilities/Shared_DeckGL.tsx
**Line:** 91:99
**Comment:**
*Incorrect Condition Logic: The tile-choice validator accepts an empty array as valid, so a deployment with `DECKGL_BASE_MAP=[]` will set `deckglTiles` to `[]` and later crash when `maplibreStyle` reads `getDeckGLTiles()[0][0]`. Tighten the validator to require at least one entry (and ideally exactly two string elements per choice) before accepting overrides, otherwise fall back to `DEFAULT_DECKGL_TILES`.
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%2F40804&comment_hash=846364058ab18d1e6b85cfdc6f9542a0fd8f67b9c8eb2e842d7d67ce437dd2b5&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40804&comment_hash=846364058ab18d1e6b85cfdc6f9542a0fd8f67b9c8eb2e842d7d67ce437dd2b5&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: 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
Re: [PR] fix(maps): preserve OSM map renderer choices [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40804:
URL: https://github.com/apache/superset/pull/40804#issuecomment-4632280719
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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
Re: [PR] fix(maps): preserve OSM map renderer choices [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40804:
URL: https://github.com/apache/superset/pull/40804#discussion_r3364231814
##########
superset-frontend/packages/superset-ui-core/src/utils/mapStyles.ts:
##########
@@ -0,0 +1,180 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+export type MapProvider = 'maplibre' | 'mapbox';
+
+export type MapStyleChoice = {
+ value: string;
+ label: string;
+ attribution?: string;
+};
+
+export type MapRendererOption = {
+ value: MapProvider;
+ label: string;
+ disabled?: boolean;
+};
+
+export type RasterTileMapStyle = {
+ version: 8;
+ sources: {
+ [sourceId: string]: {
+ type: 'raster';
+ tiles: string[];
+ tileSize: 256;
+ attribution: string;
+ };
+ };
+ layers: [
+ {
+ id: string;
+ type: 'raster';
+ source: string;
+ minzoom: 0;
+ maxzoom: 22;
+ },
+ ];
+};
+
+export type ResolvedMapStyle = string | RasterTileMapStyle;
+
+export const OSM_TILE_STYLE_URL =
+ 'https://tile.openstreetmap.org/{z}/{x}/{y}.png';
+export const OSM_TILE_ATTRIBUTION = '© OpenStreetMap contributors';
+export const OSM_TILE_STYLE_CHOICE: MapStyleChoice = {
+ value: OSM_TILE_STYLE_URL,
+ label: 'Streets (OSM)',
+ attribution: OSM_TILE_ATTRIBUTION,
+};
+
+export const MAPLIBRE_RENDERER_OPTION: MapRendererOption = {
+ value: 'maplibre',
+ label: 'MapLibre (open-source)',
+};
+export const MAPBOX_RENDERER_OPTION: MapRendererOption = {
+ value: 'mapbox',
+ label: 'Mapbox (API key required)',
+};
+export const DISABLED_MAPBOX_RENDERER_OPTION: MapRendererOption = {
+ ...MAPBOX_RENDERER_OPTION,
+ label: 'Mapbox (MAPBOX_API_KEY required)',
+ disabled: true,
+};
+
+const TILE_PROTOCOL = 'tile://';
+const RASTER_SOURCE_ID = 'osm-raster-tiles';
+const RASTER_LAYER_ID = 'osm-raster-layer';
+
+type BootstrapData = {
+ common?: {
+ conf?: {
+ MAPBOX_API_KEY?: unknown;
+ };
+ };
+};
+
+export function getBootstrapDataFromDocument(): unknown {
+ if (typeof document === 'undefined') {
+ return undefined;
+ }
+
+ try {
+ const appContainer = document.getElementById('app');
+ const dataBootstrap = appContainer?.getAttribute('data-bootstrap');
+ return dataBootstrap ? JSON.parse(dataBootstrap) : undefined;
+ } catch {
+ return undefined;
+ }
+}
+
+export function getMapboxApiKeyFromBootstrap(
+ bootstrapData: unknown = getBootstrapDataFromDocument(),
+): string {
+ const mapboxApiKey = (bootstrapData as BootstrapData | undefined)?.common
+ ?.conf?.MAPBOX_API_KEY;
+ return typeof mapboxApiKey === 'string' ? mapboxApiKey : '';
+}
+
+export function hasMapboxApiKey(
+ bootstrapData: unknown = getBootstrapDataFromDocument(),
+): boolean {
+ return getMapboxApiKeyFromBootstrap(bootstrapData).trim().length > 0;
+}
+
+export function getMapRendererOptions({
+ hasMapboxKey,
+}: {
+ hasMapboxKey: boolean;
+ currentValue?: MapProvider;
+}): MapRendererOption[] {
+ return [
+ MAPLIBRE_RENDERER_OPTION,
+ hasMapboxKey ? MAPBOX_RENDERER_OPTION : DISABLED_MAPBOX_RENDERER_OPTION,
+ ];
+}
+
+function unwrapTileProtocol(value: string): string {
+ return value.startsWith(TILE_PROTOCOL)
+ ? value.slice(TILE_PROTOCOL.length)
+ : value;
+}
+
+export function isRasterTileTemplate(value: unknown): value is string {
+ if (typeof value !== 'string') {
+ return false;
+ }
+ const tileUrl = unwrapTileProtocol(value);
+ return ['{z}', '{x}', '{y}'].every(templateParam =>
+ tileUrl.includes(templateParam),
+ );
+}
+
+export function buildRasterTileMapStyle(value: string): RasterTileMapStyle {
+ return {
+ version: 8,
+ sources: {
+ [RASTER_SOURCE_ID]: {
+ type: 'raster',
+ tiles: [unwrapTileProtocol(value)],
+ tileSize: 256,
+ attribution: OSM_TILE_ATTRIBUTION,
Review Comment:
**Suggestion:** `buildRasterTileMapStyle` hardcodes OpenStreetMap attribution for every raster tile template, including user-provided free-form URLs. This will display incorrect attribution text for non-OSM providers and can violate tile-provider attribution requirements. Pass attribution as an input (or only apply OSM attribution when the URL is the known OSM template) instead of forcing `OSM_TILE_ATTRIBUTION` for all raster styles. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Non-OSM raster tiles show incorrect OpenStreetMap attribution.
- ⚠️ Violates custom tile provider attribution requirements.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Create or migrate a Deck GL-based map chart so that its form data uses the MapLibre
renderer with a raster tile template, for example by saving chart params where
`maplibre_style` is a `tile://` URL like
`tile://https://tiles.example.com/{z}/{x}/{y}.png` (params structure and `maplibre_style`
field are exercised in
`tests/integration_tests/migrations/ce6bd21901ab_migrate_deckgl_and_mapbox__test.py:101-117`).
2. When the chart is rendered, the Deck GL factory at
`superset-frontend/plugins/preset-chart-deckgl/src/factory.tsx:194-201` passes
`formData.maplibre_style` into `DeckGLContainerStyledWrapper` as the `mapStyle` prop and
sets `mapProvider` to `'maplibre'`.
3. Inside `DeckGLContainer` at
`superset-frontend/plugins/preset-chart-deckgl/src/DeckGLContainer.tsx:11-15`, because
`mapProvider` is `'maplibre'`, it computes `mapStyle` by calling
`resolveMapStyle(props.mapStyle, DEFAULT_MAP_STYLE)`, where `props.mapStyle` is the
user-supplied tile template URL.
4. `resolveMapStyle` in
`superset-frontend/packages/superset-ui-core/src/utils/mapStyles.ts:52-60` calls
`isRasterTileTemplate(value)`; `isRasterTileTemplate` only checks for `{z}`, `{x}`, `{y}`
placeholders after unwrapping the `tile://` protocol (lines 19-27), so any
`tile://http.../{z}/{x}/{y}.png` URL (including non-OSM providers) is treated as a raster
template.
5. For such a raster template, `resolveMapStyle` calls `buildRasterTileMapStyle(value)` at
`mapStyles.ts:29-49`, which constructs a MapLibre style object whose source attribution is
hardcoded to `OSM_TILE_ATTRIBUTION` (`'© OpenStreetMap contributors'`) at
`mapStyles.ts:37`, regardless of the actual tile host.
6. The resulting style object is passed into the MapLibre map component in
`DeckGLContainer` (`DeckGLContainer.tsx:50-67`), so the rendered map displays an
OpenStreetMap attribution string even when the tiles come from a non-OSM provider, leading
to incorrect and potentially non-compliant attribution for user-configured tile servers.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=39bed20fe4a94aa7898184773e89fb46&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=39bed20fe4a94aa7898184773e89fb46&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(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-frontend/packages/superset-ui-core/src/utils/mapStyles.ts
**Line:** 156:156
**Comment:**
*Logic Error: `buildRasterTileMapStyle` hardcodes OpenStreetMap attribution for every raster tile template, including user-provided free-form URLs. This will display incorrect attribution text for non-OSM providers and can violate tile-provider attribution requirements. Pass attribution as an input (or only apply OSM attribution when the URL is the known OSM template) instead of forcing `OSM_TILE_ATTRIBUTION` for all raster styles.
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%2F40804&comment_hash=659d6a4f15deada96d7bf45a063654cdbd4a24911b526256e053c8a89d0e6341&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40804&comment_hash=659d6a4f15deada96d7bf45a063654cdbd4a24911b526256e053c8a89d0e6341&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: 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
Re: [PR] fix(maps): preserve OSM styles and configurable renderer defaults [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40804:
URL: https://github.com/apache/superset/pull/40804#issuecomment-4644427612
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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
Re: [PR] fix(maps): preserve OSM styles and configurable renderer defaults [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
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: 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
Re: [PR] fix(maps): preserve OSM styles and configurable renderer defaults [superset]
Posted by "richardfogaca (via GitHub)" <gi...@apache.org>.
richardfogaca commented on PR #40804:
URL: https://github.com/apache/superset/pull/40804#issuecomment-4663213908
Addressed the remaining map review feedback in 8df019fcbe35ed1ab4de677281656401be9718ac: guarded the deck.gl datasource metrics and color-breakpoint controls type paths, switched OSM attribution to hostname/subdomain detection, and added focused coverage for OSM subdomain plus lookalike hosts. Local frontend typecheck and focused map-style tests pass.
--
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