-
Notifications
You must be signed in to change notification settings - Fork 502
Python: Allow importing python compat flag from python source code #5713
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
base: main
Are you sure you want to change the base?
Conversation
CodSpeed Performance ReportMerging #5713 will not alter performanceComparing Summary
Footnotes
|
3200680 to
06b1e0c
Compare
06b1e0c to
1b49230
Compare
3fe54f1 to
1152c07
Compare
1152c07 to
1a46099
Compare
dom96
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code changes look good.
One question I have is how does this interact with our baseline snapshots? We have to be careful to not serialise the compat flags in that snapshot as it could then cause issues.
| | { cloudflare_compat_flags: true } | ||
| | SerializedJsModule; | ||
| /** | ||
| * Global objects that we need a custom serializer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * Global objects that we need a custom serializer | |
| * Global objects that we need a custom serializer for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about "Global objects that need a custom serializer"
| get_pyodide_entrypoint_helper() | ||
| ); | ||
|
|
||
| pyodide.registerJsModule('_cloudflare_compat_flags', COMPATIBILITY_FLAGS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bike-shedding but maybe this should be _cf_private.compat_flags just to future proof a bit, so that if we get more of these private modules, they have a nice home
What I thought was that because this uses a custom serializer, the compat flags will pick up the fresh, user-passed flags when deserializing the snapshot. @hoodmane Could you confirm that I am understanding it correctly? |
|
That's right. Though if you say |
hoodmane
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Related: #5678
We would like to make a small breaking change in Python, but it seems like currently importing compat flags are only available in the JS code for Python workers.
This PR enables importing compat flags from python code of Python workers such as
This PR includes updating the custom serializer for the snapshot as the compat flag object is exposed as a global object.