Skip to content

[DRAFT] chore: add experimental python 314 support#129

Draft
ryanking13 wants to merge 1 commit into
mainfrom
gyeongjae/experimental-python314
Draft

[DRAFT] chore: add experimental python 314 support#129
ryanking13 wants to merge 1 commit into
mainfrom
gyeongjae/experimental-python314

Conversation

@ryanking13

Copy link
Copy Markdown
Contributor

This should not be merged until we land Python 314 support to workerd.

@ask-bonk ask-bonk Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm Bonk, and I've done a quick review of your PR.

This PR adds experimental Python 3.14 support, gated behind an experimental compat flag.

Issues

  1. Missing negative test for experimental gating — The new test covers the case where experimental is present, but there is no test verifying that pythonWorkers20260610 is ignored when experimental is absent. This is the most important behavior to validate for the gating logic.

  2. Placeholder Pyodide index version 314.0.0get_pyodide_index() maps 3.14 to version 314.0.0, which is clearly a placeholder. If someone enables the experimental flag today, they'll get a broken index URL with no clear error message. Consider adding a comment explaining this is a placeholder, or raising an error until the real version is known.

  3. Compat flag naming convention — The new flag pythonWorkers20260610 uses camelCase, while existing flags (python_workers, python_workers_20250116) use snake_case. This may be intentional to match the workerd side, but it's worth a note if so.

""")

version = get_python_version()
assert version == "3.14"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This test covers the happy path, but there's no test for when pythonWorkers20260610 is present without the experimental flag. That's the key scenario the gating logic protects against — please add a negative test to verify it falls back to an earlier version.

Suggested change
assert version == "3.14"
version = get_python_version()
assert version == "3.14"
def test_314_compat_flag_without_experimental(test_dir):
"""Test that Python 3.14 is NOT selected without the experimental flag."""
wrangler_toml = test_dir / "wrangler.toml"
wrangler_toml.write_text("""
name = "test-worker"
compatibility_flags = ["python_workers", "pythonWorkers20260610"]
compatibility_date = "2026-06-10"
""")
version = get_python_version()
assert version == "3.12"

case "3.13":
v = "0.28.3"
case "3.14":
v = "314.0.0"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

314.0.0 is clearly a placeholder. If someone enables the experimental flag today, pywrangler sync will silently try to fetch from a non-existent index URL. Consider adding a comment, or raising an explicit error until a real Pyodide build for 3.14 is available.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

experimental flag will fail on publish for non-Cloudflare employees so it is safe.

@ask-bonk

ask-bonk Bot commented Jun 18, 2026

Copy link
Copy Markdown

Review posted successfully on PR #129. Here's a summary of what I flagged:

  1. Missing negative test (highest priority) — There's no test verifying that pythonWorkers20260610 is ignored when the experimental flag is absent. I posted a suggestion with a concrete test case at packages/cli/tests/test_py_version_detect.py:190.

  2. Placeholder Pyodide index version314.0.0 in get_pyodide_index() will silently produce a broken URL if someone enables experimental mode today. Flagged at packages/cli/src/pywrangler/utils.py:417.

  3. Compat flag naming convention — The new pythonWorkers20260610 flag uses camelCase unlike existing snake_case flags. Noted in the review body for the author to confirm if intentional.

github run

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.

1 participant