Pin workerd local dev python package version numbers to match deployment #1707
Conversation
garrettgu10
changed the title
garrettgu10
changed the title
|
I couldn't find the version numbers for async_timeout or setuptools. @dom96 do you know what these are? |
There was a problem hiding this comment.
Thanks @garrettgu10 this looks good to me for now. Eventually it would be nice to extract this from pyodide_packages.tar or something like that though since these will take some effort to update.
As far as I can tell we aren't including them anymore and that was just out of date. |
| } | ||
|
|
||
| function addPinnedVersionToRequirement(requirement) { | ||
| if (requirement.includes("==")) { |
There was a problem hiding this comment.
We should move this to the C++ and be even more strict: only allow characters which are valid package identifiers (so no spaces, no <, >, =, etc).
A good place for this would be workerd-api (and the equivalent in EW).
There was a problem hiding this comment.
Feel free to do this in a separate PR.
There was a problem hiding this comment.
I think it would be better to check for presence of the requirement against the lock file. That would also catch package names that we don't have / have a typo.
There was a problem hiding this comment.
@hoodmane that would come after we disable micropip on edgeworker, right?
| "anyio": "3.7.1", | ||
| "attrs": "23.2.0", | ||
| "certifi": "2024.2.2", | ||
| "charset_normalizer": "3.3.2", |
There was a problem hiding this comment.
I kept the underscores since that's what the original code said, but I think it might be necessary to normalize package names in this list as well as during lookup, since some users may use e.g. langchain-community in requirements.txt even though the name is langchain_community at import time.
But that might be better to put in another PR.
There was a problem hiding this comment.
We probably want to have pretty similar logic to recursiveDependencies here:
https://github.com/pyodide/pyodide/blob/main/src/js/load-package.ts?plain=1#L186
In fact there's a pretty good chance we could use that same logic unchanged.
There was a problem hiding this comment.
Good point, yeah, normalization is a must.
garrettgu10
force-pushed
the
ggu/python-requirement-version-pinning
branch
from
1b324da
to
4042bc3
Compare
During local development, we would like micropip to fetch the same version numbers as what's currently deployed on edgeworker. This PR hardcodes some version numbers used by edgeworker according to (https://github.com/dom96/pyodide_packages/releases) and makes sure that they are passed to micropip when installing the packages.