-
Notifications
You must be signed in to change notification settings - Fork 328
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
Add compatibility date for Pyodide 0.27.0 #3268
base: main
Are you sure you want to change the base?
Conversation
45c5971
to
76beac7
Compare
@@ -673,4 +673,11 @@ struct CompatibilityFlags @0x8f8c1b68151b6cef { | |||
tailWorkerUserSpans @69 :Bool | |||
$compatEnableFlag("tail_worker_user_spans") | |||
$experimental; | |||
|
|||
pythonWorkersV2 @70 :Bool |
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.
Again the V2 looks wrong to me, I don't know if that's the way we should go down.
We'll eventually have v2, v3, v4, v5 ... but that's not very descriptive of what they are and why the user should use one or the other.
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.
I was thinking that we'll just add a bunch of $impliedByAfterDate
's and the user passes python_workers
and a date.
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.
In any case, I think it's okay to change the name as long as it's marked experimental. I also don't think "pyodide v0.26.4" means that much to the end users. Calendar versioning would be reasonable like pythonWorkers_2024_12
.
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.
Yeah calendar versioning seems better than pyodide versioning, I agree the pyodide version doesn't tell the user much
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.
My only reluctance is that I don't see any compat flags with underscores in their names. But camel case doesn't really work for dates...
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.
Does experimental
just stop the user from setting the flag directly with the enable flag and still allow it to be set by implies_by_after_date?
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.
Anyways, if that doesn't work we can always remove both experimental
and the enable_flag
once we add an impliedByAfterDate
.
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.
Does experimental just stop the user from setting the flag directly
yes.
still allow it to be set by implies_by_after_date?
I don't know, but if not then we need another approach here.
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.
doesn't work we can always remove both
experimental
Yeah that would work, add that to the documentation above
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.
The contract for experimental
flags is that only accounts with a @cloudflare.com email can use those flags. Even if we can get around that mechanism by using impliesByAfterDate
we shouldn't do so (and if that's possible then arguably we should fix it to avoid accidents). We should remove experimental
and enableFlag
instead as Hood suggested.
521fe8c
to
379fb1e
Compare
379fb1e
to
15342dd
Compare
15342dd
to
8d90a57
Compare
No description provided.