Conversation
62fdc3e to
51632f2
Compare
Co-authored-by: Hood Chatham <roberthoodchatham@gmail.com>
281902c to
e556b73
Compare
|
One point of jank is that the tip-of-tree pyodide bundle is still named |
…k cache, using generated tip-of-tree pyodide to pre-populate disk cache
ada41a7 to
0e324a8
Compare
|
Another thing we need to figure out before merging -- how can we determine if a worker is a Python worker or not? We need to know this so that we don't fetch the Pyodide bundle when the worker doesn't need it. |
|
Since it's blocking, why not call |
This story is complete with #2464. You should be able to use it to determine the bundle filename to download.
By checking the WorkerBundle. We've got the |
|
@dom96 ah, makes sense. We should probably copy pyodide-2.capnp.bin to pyodide-0.26.0a2.capnp.bin on our bucket so we can use that. I'll do that |
dom96
left a comment
There was a problem hiding this comment.
LGTM but I think you should integrate PythonSnapshotRelease into here before merging. You probably want to move getPythonSnapshotRelease out of EW and into workerd, then use it here to get the right Python version to load.
|
Could you split "refactor bazel test setup" out into a separate PR? |
e9d6b11 to
c312b50
Compare
…ternal autogate on all python tests
96fef97 to
c3d0a86
Compare
| if (fetchPyodideBundle(impl->pythonConfig, "dev"_kj)) { | ||
| modules->addBuiltinBundle(getPyodideBundle("dev"_kj), kj::none); | ||
| } else { |
There was a problem hiding this comment.
We'll want to select between "dev" and the appropriate version by setting an appropriate compat flag so that tests can be a bit more declarative. I think it would be confusing if you try to test a specific version but the test uses latest because it exists. But we can deal with that in a followup.
| } | ||
| } | ||
|
|
||
| bool fetchPyodideBundle(const api::pyodide::PythonConfig& pyConfig, kj::StringPtr version) { |
There was a problem hiding this comment.
I think it would be tidier to have fetchPyodideBundle return a kj::Maybe<> directly, but we can tidy up that sort of thing in followup PRs.
hoodmane
left a comment
There was a problem hiding this comment.
Thanks @garrettgu10! Let's merge this.
This loads the Pyodide bundle through HTTP. It does this in a synchronous way, by creating a new kj::Thread and immediately disowning it, causing the current thread to be blocked on the completion of the new thread.
When a disk cache directory is provided at the command line using pyodide-bundle-disk-cache, the directory is used to cache pyodide bundles.
Existing test cases have been edited to use the
pyodide.capnp.binfile generated through bazel, so changes insrc/pyodidewill be reflected instantly if tested throughbazel test .... Otherwise, the correct version according to compat date will be fetched from the internet.Here's a plan for how to test the disk cache writing (reading is covered by bazel tests) functionality: