Skip to content

[Python] Remove all to_js usage from bindings examples#31636

Open
ryanking13 wants to merge 1 commit into
productionfrom
python-workers-ga/to-js-removal
Open

[Python] Remove all to_js usage from bindings examples#31636
ryanking13 wants to merge 1 commit into
productionfrom
python-workers-ga/to-js-removal

Conversation

@ryanking13

Copy link
Copy Markdown
Contributor

Summary

Removes remaining pyodide.ffi.to_js usage from workers bindings documentation. Python workers team has improved the SDK so that users does not have to manually convert Python objects into JS-serializable object before passing to RPC boundary.

Screenshots (optional)

Documentation checklist

@github-actions github-actions Bot added product:workers Related to Workers product product:queues Cloudflare Queues: https://developers.cloudflare.com/queues product:workflows Workflows: https://developers.cloudflare.com/workflows/ size/m labels Jun 23, 2026
@cloudflare-docs-bot

cloudflare-docs-bot Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Review

🚨 1 critical, ⚠️ 5 warnings found in commit bee16e1.

Code Review

This code review is in beta and may not always be helpful — use your judgment.

Critical (1)
File Issue
workers/examples/modify-request-property.mdx line 171 Mixed tabs and spaces in Python example — The added comment line uses tabs (\t\t\t\t\t\t) for indentation while the surrounding dictionary block uses spaces. Python raises TabError: inconsistent use of tabs and spaces in indentation when a user copies and runs this code. Fix: Replace the leading tabs with 12 spaces to match the surrounding dictionary indentation.
Warnings (5)
File Issue
workers/examples/cors-header-proxy.mdx line 563 Response status lost when recreating responseResponse(response.body, headers=dict(response.headers)) constructs a new Response without copying the upstream status or status_text, so any non-200 response from the proxied API will be rewritten as 200 OK. Fix: Pass the original status and status text when reconstructing the response, e.g. Response(response.body, status=response.status, status_text=response.status_text, headers=response.headers).
workers/examples/cache-using-fetch.mdx line 154 Reconstructed Response drops upstream status — Python rebuilds the response with only headers=dict(response.headers), so the upstream HTTP status code and status text default to 200/OK. The JavaScript equivalent passes the original response object as the init argument, which also carries over status and statusText. Fix: Forward the original status and status text when constructing the new Response, e.g. Response(response.body, status=response.status, status_text=response.status_text, headers=dict(response.headers)), using the attribute names exposed by the Python runtime.
workflows/python/bindings.mdx line 103 Mixed tabs and spaces in Python example — The added comment line begins with three tab characters (\t\t\t) while the surrounding async def fetch body and the following instances = [ line use spaces. Python raises TabError: inconsistent use of tabs and spaces in indentation for this block. Fix: Replace the leading tabs on line 103 with 8 spaces so the comment aligns with the rest of the method body.
workers/examples/cache-tags.mdx line 169 Unhandled fetch failure — The awaited fetch(url, cf={"cacheTags": tags}) has no error handling. If the upstream request fails or the URL is invalid, the exception propagates instead of returning a JSON 500 response like the JavaScript and Hono tabs. Fix: Wrap the fetch call in a try/except and return Response.json({"error": ...}, status=500) on failure, matching the other language tabs.
workers/languages/python/examples.mdx line 151 Inconsistent indentation in Python example — The added comment line in the 'Publish to a Queue' example is indented with tab characters (\t\t\t) while the surrounding method body uses spaces. Copying the snippet will cause Python to raise a TabError/IndentationError. Fix: Replace the leading tabs with 8 spaces so the comment matches the surrounding fetch body indentation.

Style Guide Review

✅ No style-guide issues found.

Commands

Only codeowners can run commands. Post a comment with the command to trigger it.

Command Description
/review Runs a review now. Incremental if a prior review exists, full if not.
/full-review Re-reviews the entire PR diff from scratch, ignoring incremental history. Useful after a rebase, when you want a fresh review, or if the bot gets out of sync and reports issues that no longer exist.
/ignore-review-limit Permanently lifts the 2-review automatic limit for this PR. Future pushes will trigger reviews as normal.

@github-actions

Copy link
Copy Markdown
Contributor

This pull request requires reviews from CODEOWNERS as it changes files that match the following patterns:

Pattern Owners
/src/content/docs/queues/ @elithrar, @jonesphillip, @harshil1712, @mia303, @cloudflare/product-owners
/src/content/docs/workers/ @cloudflare/workers-docs, @GregBrimble, @irvinebroque, @mikenomitch, @korinne, @WalshyDev, @cloudflare/deploy-config, @cloudflare/product-owners, @cloudflare/wrangler, @MattieTK, @cloudflare/dev-plat-leads, @vy-ton
/src/content/docs/workflows/ @elithrar, @rita3ko, @irvinebroque, @vy-ton, @celso, @deloreyj, @mia303, @jonesphillip, @cloudflare/product-owners

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.

This should be merged after cloudflare/workers-py#138 is merged and released

api_url2 = api_url

target = urlparse(api_url2)
request = Request.new(api_url2, request)

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.

@dom96 @hoodmane

We don't have a corresponding method for this (cloning the Response with a new URL). I guess we should add it in our SDK

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Request has a clone method, maybe we can give it an optional URL argument?

@github-actions

Copy link
Copy Markdown
Contributor

Preview URL: https://bee16e18.preview.developers.cloudflare.com
Preview Branch URL: https://python-workers-ga-to-js-removal.preview.developers.cloudflare.com

Files with changes (up to 15)

Original Link Updated Link
https://developers.cloudflare.com/workers/examples/post-json/ https://python-workers-ga-to-js-removal.preview.developers.cloudflare.com/workers/examples/post-json/
https://developers.cloudflare.com/workers/examples/cors-header-proxy/ https://python-workers-ga-to-js-removal.preview.developers.cloudflare.com/workers/examples/cors-header-proxy/
https://developers.cloudflare.com/workflows/python/bindings/ https://python-workers-ga-to-js-removal.preview.developers.cloudflare.com/workflows/python/bindings/
https://developers.cloudflare.com/workers/examples/modify-request-property/ https://python-workers-ga-to-js-removal.preview.developers.cloudflare.com/workers/examples/modify-request-property/
https://developers.cloudflare.com/workers/examples/cache-using-fetch/ https://python-workers-ga-to-js-removal.preview.developers.cloudflare.com/workers/examples/cache-using-fetch/
https://developers.cloudflare.com/workers/examples/cache-tags/ https://python-workers-ga-to-js-removal.preview.developers.cloudflare.com/workers/examples/cache-tags/
https://developers.cloudflare.com/workers/languages/python/examples/ https://python-workers-ga-to-js-removal.preview.developers.cloudflare.com/workers/languages/python/examples/
https://developers.cloudflare.com/queues/configuration/javascript-apis/ https://python-workers-ga-to-js-removal.preview.developers.cloudflare.com/queues/configuration/javascript-apis/
https://developers.cloudflare.com/workflows/python/python-workers-api/ https://python-workers-ga-to-js-removal.preview.developers.cloudflare.com/workflows/python/python-workers-api/

@dom96 dom96 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, thanks for fixing these!

return await fetch(new_request)
except Exception as e:
return Response.new({"error": str(e)}, status=500)
return Response(str(e), status=500)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wonder if the idea here is to return json. Maybe you can use Response.json here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

product:queues Cloudflare Queues: https://developers.cloudflare.com/queues product:workers Related to Workers product product:workflows Workflows: https://developers.cloudflare.com/workflows/ size/m

Projects

None yet

Development

Successfully merging this pull request may close these issues.