Skip to content

feat(notebook-migration, local-dev): manage the notebook migration service and JupyterLab - #7934

Merged
mengw15 merged 4 commits into
apache:mainfrom
zyratlo:migration-tool-local-dev
Aug 26, 2026
Merged

feat(notebook-migration, local-dev): manage the notebook migration service and JupyterLab#7934
mengw15 merged 4 commits into
apache:mainfrom
zyratlo:migration-tool-local-dev

Conversation

@zyratlo

@zyratlo zyratlo commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

bin/local-dev.sh had no knowledge of the notebook migration tool, so a contributor running the local dev stack got the tool in the GUI with nothing serving it. This PR registers both pieces with the local dev tooling. It also fixes the JupyterLab iframe, which the browser refused to render under local dev because the origin allowed to frame it was inherited from the single-node deployment.

bin/local-dev/main.sh (service catalog)

  • Registers notebook-migration-service as a managed JVM service on port 9098, following the same catalog shape as the other single module services. No launch case is needed because start_one is generic over the catalog, and no dirty-source entry is needed because _precompute_src_dirs derives the transitive source closure from the sbt project name.
  • Registers jupyter as Docker infrastructure on port 9100 and adds it to the sets that up starts and that state reporting reads, so it appears in the dashboard rather than only starting silently.
  • The startup summary counted containers with a literal that is now wrong. It reads the length of the long lived infrastructure list instead, so it cannot drift again.

bin/local-dev/tui.py (dashboard catalog)

  • Adds the matching rows so both services appear in the interactive dashboard. Order matches main.sh, whose array is documented as start order.

bin/local-dev/docker-compose.override.yml (iframe origin)

  • Points TEXERA_ORIGIN at the Angular dev server (http://localhost:4200) instead of the nginx origin the single-node stack sets (http://localhost:8080).
  • That value is what start-texera-jupyter.sh feeds into Jupyter's frame-ancestors CSP directive, so under local dev the only origin permitted to frame the notebook was one that never frames it. The browser refused to render the panel: Firefox reports that localhost:9100 will not allow the page to be displayed if another site has embedded it, and Chrome blocks it less explicitly.
  • The same value is also substituted into custom.js as the postMessage targetOrigin and the inbound origin check, so the cell click sync between the workspace panel and the notebook was silently broken as well. One variable fixes both.
  • This is the same class of gap the file already corrects for this deployment mode, alongside publishing the infrastructure ports to the host and rewriting lakekeeper-init's STORAGE_S3_ENDPOINT away from the in network hostname. It is defaulted rather than hardcoded, matching that entry's style, so a deployment under a real hostname can still override it.

bin/local-dev/README.md

  • Documents rebuilding the JupyterLab image. It is the only managed service running from a Texera built image rather than natively, so edits to its customizations do nothing until it is rebuilt. Also notes deleting the local tag afterwards, since it otherwise shadows the published image and the contributor keeps running a stale build.

Any related issues, documentation, discussions?

Closes #7933
Parent issue #4301

Depends on #7932, which defines the jupyter compose service and its image.

How was this PR tested?

bin/local-dev/tests/test_local_dev_sh.sh passes (117). The catalog invariants in test_local_dev_tui.py run in CI's infra job.

Started both services through the tooling against a checkout that also carries #7932. up jupyter brings the container up healthy with /api reachable on 9100; up notebook-migration-service builds and launches it natively, with /api/healthcheck returning ok and the authenticated routes returning 401.

For the origin fix, confirmed Jupyter serves frame-ancestors http://localhost:4200 and that custom.js carries the same origin. The compose merge yields one value, not a duplicate key: single-node alone still resolves to http://localhost:8080.

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Claude Opus 5)

@github-actions github-actions Bot added docs Changes related to documentations infra labels Aug 24, 2026
@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • Contributors with relevant context: @Yicong-Huang, @tanishqgandhi1908
    You can notify them by mentioning @Yicong-Huang, @tanishqgandhi1908 in a comment.

@github-actions github-actions Bot added ci changes related to CI common platform Non-amber Scala service paths labels Aug 25, 2026
@zyratlo
zyratlo force-pushed the migration-tool-local-dev branch from 7bfa78a to bd0c0ad Compare August 25, 2026 21:04
@github-actions github-actions Bot removed ci changes related to CI common platform Non-amber Scala service paths labels Aug 25, 2026
@mengw15
mengw15 requested a balanced review from Copilot August 25, 2026 22:26

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.

Copilot wasn't able to review any files in this pull request.


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@zyratlo
zyratlo marked this pull request as ready for review August 25, 2026 23:17
@zyratlo

zyratlo commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

/request-review @mengw15

@github-actions
github-actions Bot requested a review from mengw15 August 25, 2026 23:17
@mengw15
mengw15 requested a balanced review from Copilot August 26, 2026 00:30

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.

Copilot wasn't able to review any files in this pull request.

@mengw15 mengw15 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.

LGTM

@mengw15
mengw15 added this pull request to the merge queue Aug 26, 2026
Merged via the queue into apache:main with commit 9fe0657 Aug 26, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Changes related to documentations infra

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Notebook Migration] Manage the notebook migration service and JupyterLab in the local dev tooling

3 participants