Skip to content

fix: route local tile URLs through jupyter-loopback in get_local_tile_url#1337

Merged
giswqs merged 2 commits into
masterfrom
fix-issue-1331
Jul 4, 2026
Merged

fix: route local tile URLs through jupyter-loopback in get_local_tile_url#1337
giswqs merged 2 commits into
masterfrom
fix-issue-1331

Conversation

@giswqs

@giswqs giswqs commented Jul 4, 2026

Copy link
Copy Markdown
Member

Summary

Fixes #1331.

Map.add_raster in maplibregl.py uses common.get_local_tile_url, which builds a bare TileClient and calls client.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_layerget_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)

  • Call client.enable_jupyter_loopback() after creating the TileClient, guarded with hasattr/try so it degrades gracefully on older localtileserver versions (the method is new in localtileserver 1.0.0).
  • Align LOCALTILESERVER_CLIENT_PREFIX handling with get_local_tile_layer: only override the env var when it is unset (respect a user-set value), and give the prefix kwarg final precedence instead of gating it behind an elif that skipped it on Studio Lab / AWS.

Testing

Verified in the geo conda env (localtileserver 0.11.0):

  • No regression — get_local_tile_url still returns a valid tile URL; the loopback call is skipped since the method is absent.
  • With a stubbed enable_jupyter_loopback injected onto TileClient (simulating 1.0.0+), the method is invoked exactly once during get_local_tile_url.

Summary by CodeRabbit

  • Bug Fixes
    • Improved local tile loading in notebook environments by routing requests through the Jupyter loopback path when supported.
    • Preserved any pre-existing local tile server prefix configuration instead of overwriting it automatically.
    • Kept prefix argument precedence over environment-based settings, and avoided changes when prefix=None is provided.
  • Tests
    • Added unit coverage for loopback enablement behavior, prefix override rules, and environment variable preservation.

…_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
Copilot AI review requested due to automatic review settings July 4, 2026 20:17
@coderabbitai

coderabbitai Bot commented Jul 4, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Local tile serving now preserves existing LOCALTILESERVER_CLIENT_PREFIX values, respects an explicit prefix override, and attempts Jupyter loopback activation on TileClient before returning tile URLs. Tests cover loopback support and prefix precedence.

Changes

Local Tile Server Fixes

Layer / File(s) Summary
Prefix handling and loopback enablement
leafmap/common.py
get_local_tile_layer only sets LOCALTILESERVER_CLIENT_PREFIX when it is unset, and get_local_tile_url calls enable_jupyter_loopback() when available, converting failures into warnings.
Tile URL tests
tests/test_common.py
Added a mock localtileserver helper and tests for loopback enabling, missing loopback support, prefix precedence, and preserving an existing environment prefix when prefix=None.

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
Loading

Poem

A rabbit hops through notebook air,
With loopback paths made smooth and fair.
The prefix stays, the tiles now show,
And warnings soft, like breezes, flow.
Hop, hop — the raster finds its way.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main behavioral fix, even though it names the helper rather than add_raster directly.
Linked Issues check ✅ Passed The loopback-enabled URL path addresses the Docker/Jupyter raster access regression described in #1331.
Out of Scope Changes check ✅ Passed The prefix precedence and warning behavior are part of the stated fix and supported by the added tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-issue-1331

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mergify

mergify Bot commented Jul 4, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) in get_local_tile_url() so browsers that cannot reach 127.0.0.1:<port> can still fetch tiles.
  • Updates LOCALTILESERVER_CLIENT_PREFIX handling to (a) respect user-provided env vars and (b) give the prefix kwarg final precedence, consistent with get_local_tile_layer.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread leafmap/common.py Outdated
Comment thread leafmap/common.py Outdated
Comment thread leafmap/common.py Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between c25cb04 and d9641bb.

📒 Files selected for processing (1)
  • leafmap/common.py

Comment thread leafmap/common.py
Comment thread leafmap/common.py Outdated
@github-actions

github-actions Bot commented Jul 4, 2026

Copy link
Copy Markdown

@github-actions github-actions Bot temporarily deployed to pull request July 4, 2026 20:22 Inactive
- 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).

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 win

Thread the legacy kwargs into TileClient, or drop the no-op handling leafmap/common.py:3538-3560 — the kwargs mutations here never reach TileClient(source, port=port, **client_args), so max_zoom/max_native_zoom are unused and prefix only changes LOCALTILESERVER_CLIENT_PREFIX via the environment. If this helper is meant to support the same options as get_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 value

Extract the shared local-tile prefix setup Both get_local_tile_layer and get_local_tile_url duplicate the same LOCALTILESERVER_CLIENT_PREFIX initialization and prefix kwarg 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 value

Blind-except concern from prior review resolved.

The bare except Exception: pass now emits warnings.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 adding stacklevel=2 would let the warning point at the caller of get_local_tile_url rather 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

📥 Commits

Reviewing files that changed from the base of the PR and between d9641bb and 8589a4d.

📒 Files selected for processing (2)
  • leafmap/common.py
  • tests/test_common.py

Comment thread tests/test_common.py
Comment on lines +110 to +117
@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}")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 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.

@github-actions github-actions Bot temporarily deployed to pull request July 4, 2026 20:34 Inactive
@giswqs giswqs merged commit d201b04 into master Jul 4, 2026
21 checks passed
@giswqs giswqs deleted the fix-issue-1331 branch July 4, 2026 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

add_raster from leafmap/maplibregl.py in container environment does not show raster in jupyter notebook

2 participants