Skip to content

feat(notebook-migration): resolve the Jupyter URL and token per user - #8032

Draft
zyratlo wants to merge 8 commits into
apache:mainfrom
zyratlo:migration-tool-jupyter-per-user
Draft

feat(notebook-migration): resolve the Jupyter URL and token per user#8032
zyratlo wants to merge 8 commits into
apache:mainfrom
zyratlo:migration-tool-jupyter-per-user

Conversation

@zyratlo

@zyratlo zyratlo commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

notebook-migration-service read one Jupyter URL and token from configuration as process-wide values. That is safe only under the per-user-pod model, where each user runs their own pod. A single global instance would hand every user the same Jupyter and the same token. This resolves both per user instead.

This is stage 2 of the staged roadmap in the architectural note on #5258. Stage 1 (#7390) and stage 4 (#7671) are merged. Stage 3 (#8006) depends on this one.

Registry table
A new user_jupyter table holds one row per user whose Jupyter has been provisioned, keyed on uid. Both the internal and public URLs are stored rather than derived at read time, matching how workflow_computing_unit.uri is the recorded source of truth for a computing unit's address. Storing them lets the deployment change its addressing scheme without a code change, which is what #8006 needs in order to choose between per-user hostnames and a path prefix.

Derived tokens
Each user's token is HMAC-SHA256(secret, uid), computed from storage.jupyter.token-secret. No credential is stored at rest, any replica of the service derives the same value, and rotation is a secret change. The service refuses to start when per-user Jupyter is on with an empty secret: an empty HMAC key is publicly known, so tokens would still look distinct per user while anyone could derive anyone else's.

Fallback gated on a flag, not on row absence
With kubernetes.jupyter-enabled off, every user resolves to the statically configured Jupyter, which is how the single-node (#7932) and local-dev (#7934) deployments run one shared JupyterLab. With it on, a user with no row has nothing provisioned yet and gets an unavailable response. Falling back to the shared server there would hand an unprovisioned user somebody else's notebooks, and would let delete-notebook remove another user's file. The flag is deliberately separate from kubernetes.enabled, so a deployment can run computing units on Kubernetes without per-user Jupyter.

Provisioning
JupyterKubernetesClient mirrors the computing unit's KubernetesClient, including the hostname and subdomain pair that makes the pod FQDN resolve through a headless service. get-jupyter-iframe-url and set-notebook provision on demand; get-jupyter-url and delete-notebook resolve only, so reading a URL or deleting a file never starts a pod. A registered pod that stops answering is discarded and rebuilt, since the row would otherwise outlive the pod and point every later request at nothing. Two concurrent first requests are resolved by the primary key: the loser keeps the winner's row, which holds the same uid-derived addresses.

The uid always comes from the authenticated session and never from a request body, so one user cannot address another's Jupyter.

Two notes on the implementation. The Kubernetes client is built on demand rather than at object initialisation, unlike the computing unit's, because the single-node and local-dev deployments have no cluster to build one against and never provision. Adding io.fabric8:kubernetes-client also raises three transitive versions over Dropwizard's pins (slf4j-api to 2.0.13, jackson-dataformat-yaml and jackson-datatype-jsr310 to 2.17.0); LICENSE-binary claims are updated to match.

Out of scope, and left to #8006: the Helm templates, the NetworkPolicy, and the global Deployment. Also out of scope is reclaiming idle pods. A dead pod self-heals on next use, but nothing reaps one that is merely unused, and the row's ON DELETE CASCADE drops the registration without deleting the pod. Until #8006 supplies an ingress, kubernetes.jupyter-public-url-template defaults to empty and the browser is handed the in-cluster address.

Any related issues, documentation, discussions?

Closes #7665
Parent issue #4301

How was this PR tested?

NotebookMigrationService/test passes (72) and Config/test passes (71). Test/compile across the whole repo is clean, as is scalafmtCheckAll.

New coverage: token derivation is deterministic per uid, distinct across 50 uids, and changes with the secret; the start-up guard rejects an empty secret only when the feature is on. Resolution falls back with the feature off, returns the row with it on, returns nothing for an unregistered user, and never returns one user's Jupyter to another. Provisioning is exercised against a stubbed Kubernetes client across every branch: fresh provision, reuse of a live pod, rebuild of a pod that stopped answering, cleanup when readiness never arrives, public URL templating, and the concurrent-registration race.

The existing withFakeJupyter suite is untouched and still passes on the stub bound to localhost:9100, which is the point of the flag-gated fallback: test users have no registry row and take the configured path exactly as before.

Applied sql/texera_ddl.sql to a scratch database the way CI does, confirmed the table shape, and confirmed that deleting a user removes their registration row. check_binary_deps.py passes under both the PR-mode and the strict nightly invocations, and NOTICE-binary matches generator output byte for byte.

Not tested on a live cluster. The chart has no notebook-migration or Jupyter templates yet, and the in-cluster pod address does not resolve from outside the cluster, so end to end Kubernetes validation belongs to #8006.

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

Generated-by: Claude Code (Claude Opus 5)

@github-actions github-actions Bot added dependencies Pull requests that update a dependency file ddl-change Changes to the TexeraDB DDL common platform Non-amber Scala service paths labels Aug 27, 2026
@github-actions

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: @tanishqgandhi1908, @aicam, @Ma77Ball
    You can notify them by mentioning @tanishqgandhi1908, @aicam, @Ma77Ball in a comment.

@codecov-commenter

codecov-commenter commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.25806% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.41%. Comparing base (1996448) to head (bd63ade).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...pache/texera/service/util/JupyterProvisioner.scala 83.33% 0 Missing and 10 partials ⚠️
...a/service/resource/NotebookMigrationResource.scala 94.11% 0 Missing and 1 partial ⚠️
.../apache/texera/service/util/JupyterEndpoints.scala 83.33% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8032      +/-   ##
============================================
- Coverage     93.42%   93.41%   -0.01%     
- Complexity     4704     4740      +36     
============================================
  Files          1180     1186       +6     
  Lines         47766    47895     +129     
  Branches       5321     5337      +16     
============================================
+ Hits          44624    44742     +118     
+ Misses         1699     1698       -1     
- Partials       1443     1455      +12     
Flag Coverage Δ *Carryforward flag
access-control-service 81.00% <ø> (ø)
agent-service 99.32% <ø> (ø) Carriedforward from 1996448
amber 89.58% <100.00%> (+<0.01%) ⬆️
computing-unit-managing-service 73.67% <ø> (ø)
config-service 86.73% <ø> (ø)
file-service 87.46% <ø> (ø)
frontend 95.89% <ø> (ø) Carriedforward from 1996448
notebook-migration-service 83.15% <91.72%> (+3.84%) ⬆️
pyamber 97.78% <ø> (ø) Carriedforward from 1996448
workflow-compiling-service 77.19% <ø> (ø)

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

🟢 2 better · 🔴 3 worse · ⚪ 10 noise (<±5%) · 0 without baseline

Compared against main 1996448 benchmarked on this same runner, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.

Dashboard · Run

config throughput MB/s latency max Δ latest / 7d
🔴 bs=10 sw=10 sl=64 413 0.252 23,322/32,373/32,373 us 🔴 +12.8% / 🔴 +131.1%
🔴 bs=100 sw=10 sl=64 890 0.543 110,031/152,933/152,933 us 🔴 +18.1% / 🔴 +60.8%
bs=1000 sw=10 sl=64 1,058 0.646 938,165/1,016,644/1,016,644 us ⚪ within ±5% / 🔴 -10.8%
Baseline details

Latest main 1996448 from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 413 tuples/sec 431 tuples/sec 893.99 tuples/sec -4.2% -53.8%
bs=10 sw=10 sl=64 MB/s 0.252 MB/s 0.263 MB/s 0.546 MB/s -4.2% -53.8%
bs=10 sw=10 sl=64 p50 23,322 us 20,669 us 11,176 us +12.8% +108.7%
bs=10 sw=10 sl=64 p95 32,373 us 35,498 us 14,009 us -8.8% +131.1%
bs=10 sw=10 sl=64 p99 32,373 us 35,498 us 16,898 us -8.8% +91.6%
bs=100 sw=10 sl=64 throughput 890 tuples/sec 918 tuples/sec 1,157 tuples/sec -3.1% -23.1%
bs=100 sw=10 sl=64 MB/s 0.543 MB/s 0.56 MB/s 0.706 MB/s -3.0% -23.1%
bs=100 sw=10 sl=64 p50 110,031 us 105,729 us 88,763 us +4.1% +24.0%
bs=100 sw=10 sl=64 p95 152,933 us 129,502 us 95,096 us +18.1% +60.8%
bs=100 sw=10 sl=64 p99 152,933 us 129,502 us 102,854 us +18.1% +48.7%
bs=1000 sw=10 sl=64 throughput 1,058 tuples/sec 1,044 tuples/sec 1,187 tuples/sec +1.3% -10.8%
bs=1000 sw=10 sl=64 MB/s 0.646 MB/s 0.637 MB/s 0.724 MB/s +1.4% -10.8%
bs=1000 sw=10 sl=64 p50 938,165 us 947,747 us 874,968 us -1.0% +7.2%
bs=1000 sw=10 sl=64 p95 1,016,644 us 1,046,968 us 917,815 us -2.9% +10.8%
bs=1000 sw=10 sl=64 p99 1,016,644 us 1,046,968 us 949,868 us -2.9% +7.0%
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,483.76,200,128000,413,0.252,23322.00,32373.38,32373.38
1,100,10,64,20,2247.45,2000,1280000,890,0.543,110031.09,152932.59,152932.59
2,1000,10,64,20,18906.43,20000,12800000,1058,0.646,938165.43,1016643.92,1016643.92

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

common ddl-change Changes to the TexeraDB DDL dependencies Pull requests that update a dependency file platform Non-amber Scala service paths

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Notebook Migration] Resolve Jupyter URL and token per user

2 participants