Skip to content

Move vendor dir in Python search path just before site-packages. #4204

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 16, 2025

Conversation

dom96
Copy link
Contributor

@dom96 dom96 commented May 27, 2025

We have two functions that adjust the sys path in Python Workers. The first one (adjustSysPath) only runs when not restoring from a snapshot, so it is effectively immortalised in the memory snapshots. To avoid needing to regenerate all the memory snapshots (which for package snapshots would be a problem) we just rearrange the path at the same time as when we add the vendor path to the sys path.

Test Plan

$ bazel run @workerd//src/workerd/server/tests/python:vendor_dir_development@

Python sys.path: ['/session', '/lib/python312.zip', '/lib/python3.12', '/lib/python3.12/lib-dynload', '/session/metadata', '/session/metadata/vendor', '/lib/python3.12/site-packages', '/session/lib/python3.12/site-packages']

@dom96 dom96 force-pushed the dominik/change-python-vendor-path branch from 7cb6a01 to fca27a6 Compare May 27, 2025 10:08
@danlapid
Copy link
Collaborator

/windsurf-review

@bhatiarajesh
Copy link

/windsurf review

@dom96 dom96 force-pushed the dominik/change-python-vendor-path branch 2 times, most recently from 97c8ece to 29025cc Compare June 6, 2025 16:27
@dom96 dom96 requested a review from hoodmane June 6, 2025 16:29
@dom96 dom96 marked this pull request as ready for review June 6, 2025 16:29
@dom96 dom96 requested review from a team as code owners June 6, 2025 16:29
Copy link
Contributor

@windsurf-bot windsurf-bot bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 To request another review, post a new comment with "/windsurf-review".

@dom96 dom96 force-pushed the dominik/change-python-vendor-path branch 4 times, most recently from 5d5de58 to ec91689 Compare June 13, 2025 15:28
Copy link
Contributor

@hoodmane hoodmane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@dom96
Copy link
Contributor Author

dom96 commented Jun 13, 2025

@hoodmane Hmm, looks like adjustSysPath needs to be called before the memory snapshot is collected (because site-packages need to be in the path for the package imports to work). I just reverted to its original. I guess proper thing to do is to add the site-packages path before memory snapshot is collected, but not the vendor dir. Though I'm not sure there is much point in doing this only in 0.27.7+.

@dom96 dom96 force-pushed the dominik/change-python-vendor-path branch from ec91689 to 091a6e7 Compare June 13, 2025 18:27
@dom96 dom96 merged commit 58262a1 into main Jun 16, 2025
20 checks passed
@dom96 dom96 deleted the dominik/change-python-vendor-path branch June 16, 2025 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants