Set Pyodide 314.0.0a1 as a development version#6683
Conversation
4c5f677 to
166a5fb
Compare
There was a problem hiding this comment.
I'm Bonk, and I've done a quick review of your PR.
This PR adds Pyodide 314.0.0a1 as the new development Python version (3.14), updating the build system, TypeScript pool setup, C++ server code, and tests to support the new version.
Findings (ranked by severity):
-
[MEDIUM]
build/python_metadata.bzl:214— vendored test packages for 314.0.0a1 (Python 3.14.2) reuse abi"3.13"with identical hashes to 0.28.2 entries. If Python 3.14 breaks C-ABI compatibility with 3.13, these packages won't work. Intentional reuse for testing? -
[MEDIUM]
src/workerd/server/server.c++:5836— hardcoded version allowlist for package preloading is fragile; every new Pyodide version requires updating this check. -
[LOW]
src/pyodide/internal/const.ts:8— naming inconsistency:V314_0_0omits thea1suffix, unlikeV0_26_0a2which includesa2. -
[LOW]
src/workerd/api/pyodide/setup-emscripten.c++:25-28— the rejected-promise error log is helpful for debugging, but the unconditionalKJ_ASSERT(FULFILLED)on line 31 will crash immediately after. Consider including the rejection reason in the assert message so it survives in crash logs.
This review was generated by an AI assistant and may contain inaccuracies.
|
Review posted successfully on PR #6683. Here's a summary of the findings:
No security issues, backward compatibility violations, or blocking problems found. The change is well-structured with proper version gating, test skips for features not yet supported, and the experimental flag is appropriately used. |
|
The generated output of Full Type Diff |
| // directories in `pyodideRootDir` and find one that starts with a 0 or 3 | ||
| const dir = readdirSync(pyodideRootDir).filter( | ||
| (x) => | ||
| // Support both 0.x and 314.x versions | ||
| x.startsWith('0') || x.startsWith('3') |
There was a problem hiding this comment.
Maybe we should figure out now how to get bazel to pass the version into the build. Maybe just write it into a file with a write_file() step and then read it with fs?
There was a problem hiding this comment.
Sounds good. I guess claude can figure out how to do it.
| if ( | ||
| version === PyodideVersion.V0_26_0a2 || | ||
| version === PyodideVersion.V314_0_0 | ||
| ) { |
There was a problem hiding this comment.
Better fix this before we release a real version.
There was a problem hiding this comment.
Do we have a working PR upstream?
| @@ -176,6 +177,12 @@ _REPLACEMENTS = [ | |||
| "globalThis._createPyodideModule = _createPyodideModule;", | |||
| "", | |||
| ], | |||
| # for 314 or later, pyodide.asm.mjs is es6 module | |||
| [ | |||
| "export default _createPyodideModule;", | |||
| # still expose _createPyodideModule for compatibility (import { _createPyodideModule }) | |||
| _PRELUDE + "export default _createPyodideModule; export { _createPyodideModule };", | |||
| ], | |||
There was a problem hiding this comment.
Let's add the appropriate changes in the function conditionally on the version.
There was a problem hiding this comment.
Yeah, right. Let's do that
16b3029 to
90a8e03
Compare
90a8e03 to
3dcaf0b
Compare
Updates the development Python version of Python workers to 3.14 (Pyodide 314.0.0a1).
The Pyodide version is not finalized yet, but releasing this with experimental flag would be helpful for us to debug issues with the new Python version.