fix: route local tile URLs through jupyter-loopback in get_local_tile_url#1337
Conversation
…_url maplibregl add_raster uses get_local_tile_url, which builds a bare TileClient and never enables localtileserver's jupyter-loopback bridge. As a result, tiles are served from http://127.0.0.1:<port>/ and fail to render in containerized/remote and webview-based Jupyter frontends (VS Code, Colab, Solara, marimo), unlike the ipyleaflet/folium path which enables it automatically via get_leaflet_tile_layer. Call client.enable_jupyter_loopback() when available (guarded for older localtileserver versions), and align the LOCALTILESERVER_CLIENT_PREFIX handling with get_local_tile_layer so a user-set prefix is respected and the prefix kwarg has final precedence. Fixes #1331
📝 WalkthroughWalkthroughLocal tile serving now preserves existing ChangesLocal Tile Server Fixes
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Notebook
participant get_local_tile_url
participant TileClient
participant Browser
Notebook->>get_local_tile_url: request tile URL
get_local_tile_url->>TileClient: create client
get_local_tile_url->>TileClient: enable_jupyter_loopback() if available
TileClient-->>get_local_tile_url: loopback enabled or warning path
get_local_tile_url-->>Notebook: return tile URL
Notebook->>Browser: load tiles through returned URL
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Tick the box to add this pull request to the merge queue (same as
|
There was a problem hiding this comment.
Pull request overview
This PR fixes raster rendering failures in Map.add_raster (maplibre path) for remote/containerized and webview-based Jupyter frontends by ensuring locally-served tile URLs are routed through localtileserver’s jupyter-loopback bridge and by aligning local tile URL prefix handling with the existing tile-layer helpers.
Changes:
- Enables
TileClient.enable_jupyter_loopback()(when available) inget_local_tile_url()so browsers that cannot reach127.0.0.1:<port>can still fetch tiles. - Updates
LOCALTILESERVER_CLIENT_PREFIXhandling to (a) respect user-provided env vars and (b) give theprefixkwarg final precedence, consistent withget_local_tile_layer.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@leafmap/common.py`:
- Around line 3549-3552: The LOCALTILESERVER_CLIENT_PREFIX assignment in
is_studio_lab() is a templated string, not an unused f-string, so do not apply
the raw Ruff F541 fix. Update the expression so the value still renders a single
literal {port} placeholder while removing the unnecessary f-prefix, and verify
the resulting prefix remains studiolab/default/jupyter/proxy/{port} rather than
preserving doubled braces.
- Around line 3598-3602: The `enable_jupyter_loopback()` call in the client
setup is swallowing all exceptions, which can hide real comm-bridge failures and
fall back silently. Update the `hasattr(client, "enable_jupyter_loopback")`
block to only ignore the specific unsupported-API case, and for any other
exception emit a diagnostic log with the error details instead of using a bare
`except Exception: pass`. Keep the fix localized around
`enable_jupyter_loopback` so the behavior remains explicit and debuggable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f2090e6e-2b93-442f-b144-b7cbceb5597b
📒 Files selected for processing (1)
leafmap/common.py
|
🚀 Deployed on https://6a496e56a67d2a7badf627d3--opengeos.netlify.app |
- get_local_tile_url: warn instead of silently swallowing exceptions from
enable_jupyter_loopback so genuine comm-bridge failures leave a diagnostic
trail (Copilot + CodeRabbit).
- get_local_tile_url: guard the prefix kwarg against None/non-string values so
writing LOCALTILESERVER_CLIENT_PREFIX can't raise a TypeError, and only apply
it when not None (Copilot).
- get_local_tile_url: drop the extraneous f-prefix on the Studio Lab prefix
template while keeping the literal {port} placeholder (CodeRabbit / Ruff F541).
- tests: add unit tests for get_local_tile_url covering jupyter-loopback
invocation (present and absent) and prefix precedence / None handling (Copilot).
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
leafmap/common.py (2)
3538-3560: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThread the legacy kwargs into
TileClient, or drop the no-op handlingleafmap/common.py:3538-3560— thekwargsmutations here never reachTileClient(source, port=port, **client_args), somax_zoom/max_native_zoomare unused andprefixonly changesLOCALTILESERVER_CLIENT_PREFIXvia the environment. If this helper is meant to support the same options asget_local_tile_layer, pass them through; otherwise remove this copied block.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@leafmap/common.py` around lines 3538 - 3560, The legacy kwargs handling in this helper is a no-op because the mutated kwargs never get passed into TileClient, so options like max_zoom, max_native_zoom, and prefix do not affect the client call. Update the TileClient flow in common.py to either thread these values through to TileClient(source, port=port, **client_args) or remove the copied kwargs/env-prefix block entirely if these options are not meant to be supported here. Use the TileClient call and the surrounding kwargs/prefix handling in this function to locate the fix.
3541-3560: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the shared local-tile prefix setup Both
get_local_tile_layerandget_local_tile_urlduplicate the sameLOCALTILESERVER_CLIENT_PREFIXinitialization andprefixkwarg handling; a small helper would keep that precedence logic from drifting.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@leafmap/common.py` around lines 3541 - 3560, The LOCALTILESERVER_CLIENT_PREFIX setup and prefix kwarg precedence are duplicated between get_local_tile_layer and get_local_tile_url, so extract that shared logic into a small helper and call it from both paths. Keep the same precedence order in the helper: only set the env var when unset, apply JUPYTERHUB_SERVICE_PREFIX/studiolab/AWS defaults, then let the prefix kwarg override everything. Use the existing symbols get_local_tile_layer, get_local_tile_url, and LOCALTILESERVER_CLIENT_PREFIX to centralize the behavior without changing semantics.
♻️ Duplicate comments (1)
leafmap/common.py (1)
3599-3605: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueBlind-except concern from prior review resolved.
The bare
except Exception: passnow emitswarnings.warn(...), addressing the earlier feedback that silent failures here could reintroduce the same un-proxied-URL bug this PR fixes.Ruff still flags BLE001 (broad except) and B028 (missing
stacklevel) — the broad catch is intentional (guarding an optional, version-gated API), but addingstacklevel=2would let the warning point at the caller ofget_local_tile_urlrather than this internal line.♻️ Optional: add stacklevel
except Exception as e: warnings.warn( - f"Failed to enable jupyter loopback for the local tile client: {e}" + f"Failed to enable jupyter loopback for the local tile client: {e}", + stacklevel=2, )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@leafmap/common.py` around lines 3599 - 3605, The warning in `get_local_tile_url` / the `enable_jupyter_loopback` exception handler should point to the caller instead of this internal fallback. Keep the broad `except Exception as e` guard, but update the `warnings.warn(...)` call to include `stacklevel=2` so the warning is attributed to the code that invoked `get_local_tile_url`.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_common.py`:
- Around line 110-117: The current test only covers the case where
LOCALTILESERVER_CLIENT_PREFIX is unset, so it does not verify that
get_local_tile_url’s prefix argument overrides an existing environment value.
Add a test alongside test_get_local_tile_url_prefix_precedence that starts with
LOCALTILESERVER_CLIENT_PREFIX already set, then calls get_local_tile_url with an
explicit prefix and asserts the env var ends up as the passed prefix; use
_mock_localtileserver and the get_local_tile_url helper to locate the behavior.
---
Outside diff comments:
In `@leafmap/common.py`:
- Around line 3538-3560: The legacy kwargs handling in this helper is a no-op
because the mutated kwargs never get passed into TileClient, so options like
max_zoom, max_native_zoom, and prefix do not affect the client call. Update the
TileClient flow in common.py to either thread these values through to
TileClient(source, port=port, **client_args) or remove the copied
kwargs/env-prefix block entirely if these options are not meant to be supported
here. Use the TileClient call and the surrounding kwargs/prefix handling in this
function to locate the fix.
- Around line 3541-3560: The LOCALTILESERVER_CLIENT_PREFIX setup and prefix
kwarg precedence are duplicated between get_local_tile_layer and
get_local_tile_url, so extract that shared logic into a small helper and call it
from both paths. Keep the same precedence order in the helper: only set the env
var when unset, apply JUPYTERHUB_SERVICE_PREFIX/studiolab/AWS defaults, then let
the prefix kwarg override everything. Use the existing symbols
get_local_tile_layer, get_local_tile_url, and LOCALTILESERVER_CLIENT_PREFIX to
centralize the behavior without changing semantics.
---
Duplicate comments:
In `@leafmap/common.py`:
- Around line 3599-3605: The warning in `get_local_tile_url` / the
`enable_jupyter_loopback` exception handler should point to the caller instead
of this internal fallback. Keep the broad `except Exception as e` guard, but
update the `warnings.warn(...)` call to include `stacklevel=2` so the warning is
attributed to the code that invoked `get_local_tile_url`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 42a56a92-030f-4009-9fdd-71ddb42e53bb
📒 Files selected for processing (2)
leafmap/common.pytests/test_common.py
| @patch.dict(os.environ, {}, clear=True) | ||
| def test_get_local_tile_url_prefix_precedence(self): | ||
| """The prefix kwarg takes final precedence over the env var.""" | ||
| fake_module, _ = self._mock_localtileserver() | ||
| with patch.dict("sys.modules", {"localtileserver": fake_module}): | ||
| get_local_tile_url("test.tif", prefix="proxy/{port}") | ||
| self.assertEqual(os.environ["LOCALTILESERVER_CLIENT_PREFIX"], "proxy/{port}") | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add a test for prefix overriding an already-set env var.
test_get_local_tile_url_prefix_precedence clears the environment first, so it only proves prefix is applied when nothing was previously set — it doesn't verify the "final precedence" claim (explicit prefix overriding an existing LOCALTILESERVER_CLIENT_PREFIX value), which is the core behavior this PR's commit message calls out.
`@patch.dict`(
os.environ, {"LOCALTILESERVER_CLIENT_PREFIX": "old/{port}"}, clear=True
)
def test_get_local_tile_url_prefix_overrides_existing_env(self):
"""An explicit prefix kwarg overrides an already-set env var."""
fake_module, _ = self._mock_localtileserver()
with patch.dict("sys.modules", {"localtileserver": fake_module}):
get_local_tile_url("test.tif", prefix="proxy/{port}")
self.assertEqual(os.environ["LOCALTILESERVER_CLIENT_PREFIX"], "proxy/{port}")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_common.py` around lines 110 - 117, The current test only covers
the case where LOCALTILESERVER_CLIENT_PREFIX is unset, so it does not verify
that get_local_tile_url’s prefix argument overrides an existing environment
value. Add a test alongside test_get_local_tile_url_prefix_precedence that
starts with LOCALTILESERVER_CLIENT_PREFIX already set, then calls
get_local_tile_url with an explicit prefix and asserts the env var ends up as
the passed prefix; use _mock_localtileserver and the get_local_tile_url helper
to locate the behavior.
Summary
Fixes #1331.
Map.add_rasterinmaplibregl.pyusescommon.get_local_tile_url, which builds a bareTileClientand callsclient.get_tile_url(...). Since 0.46.0 (commit 8b0e876), this path no longer enables localtileserver's jupyter-loopback comm bridge — unlike the ipyleaflet/folium path (get_local_tile_layer→get_leaflet_tile_layer), which enables it automatically.As a result, tiles are served from
http://127.0.0.1:<port>/…, which the browser cannot reach in containerized/remote and webview-based Jupyter frontends (VS Code, Colab, Solara, marimo, Docker), so the raster never renders.Changes (
leafmap/common.py,get_local_tile_url)client.enable_jupyter_loopback()after creating theTileClient, guarded withhasattr/tryso it degrades gracefully on older localtileserver versions (the method is new in localtileserver 1.0.0).LOCALTILESERVER_CLIENT_PREFIXhandling withget_local_tile_layer: only override the env var when it is unset (respect a user-set value), and give theprefixkwarg final precedence instead of gating it behind anelifthat skipped it on Studio Lab / AWS.Testing
Verified in the
geoconda env (localtileserver 0.11.0):get_local_tile_urlstill returns a valid tile URL; the loopback call is skipped since the method is absent.enable_jupyter_loopbackinjected ontoTileClient(simulating 1.0.0+), the method is invoked exactly once duringget_local_tile_url.Summary by CodeRabbit
prefixargument precedence over environment-based settings, and avoided changes whenprefix=Noneis provided.