Repository navigation
Add Pyodide patch to remove redundant Python calls during the bootstrap - #8386
ryanking13 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
💬 Approved with comments — The patch logic is well guarded and matches the upstream change. The only note is an informational one about unrelated churn in MODULE.bazel.lock, including a transitive downgrade of hermetic_launcher.
Findings: 1 info (1 is an inline comment on the diff)
The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.
📋 Code Quality: 💬 1 info
Patch logic is sound; the lockfile has unrelated transitive dependency churn.
Recommendation: 💬 Optional Nits
- [INFO] MODULE.bazel.lock downgrades hermetic_launcher from 0.0.17 to 0.0.16 and adds other entries unrelated to the Pyodide bump. (
MODULE.bazel.lock:145)
View full review
- The
finalizeBootstrappatches are strict: each one hasexpected: 1, matches an exact literal or shape, and throws when the structure doesn't match. MODULE.bazel.lockalso changes things the Pyodide bump doesn't explain:hermetic_launchergoes from 0.0.17 to 0.0.16,aspect_tools_telemetrygains 0.5.2 entries, and new extension entries appear for protobuf, rules_android and rules_rust. This usually means the lock was regenerated against an oldermainor with a different Bazel or registry state.
✅ All other checks passed
🚀 Release: ✅ LGTM
Low risk: the dependency bundle bump has updated integrity hashes.
🔒 Security: ✅ LGTM
The bundle URL host is unchanged, and the new sha256 integrity hashes are pinned. The AST rewrite runs at build time on fixed matches and takes no untrusted input.
📖 AGENTS.md: ✅ LGTM
No AGENTS.md concerns.
⚡ Performance: ✅ LGTM
Removing the Python source compilation and execution from finalizeBootstrap should reduce cold-start cost.
📝 Documentation: ✅ LGTM
No documentation concerns.
Reviewed commit: feb3cdca
hoodmane
left a comment
There was a problem hiding this comment.
Let's try to make the finalizeBootstrap: import _pyodide_core only when not restoring a snapshot patch shorter, it's a bit confusing this way.
Adds a patch to apply upstream Pyodide PR to workerd (pyodide/pyodide#6534).
This removes expensive Python AST parse / Python function calls during the bootstrap to improve the cold start.