-
Notifications
You must be signed in to change notification settings - Fork 7.3k
[doc] Python Worker handler/dict fixes #23757
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: production
Are you sure you want to change the base?
[doc] Python Worker handler/dict fixes #23757
Conversation
1. updated from string.format() to f-strings (supported since python 3.6) 2. Updated examples to be internally consistent: the params example was referencing the dict element directly whereas the json example was setting a variable from the dict 3. Updated json example to fix dicts in python do not use dot notation and reference the key instead.
correct spacing for new line int parameters example
1. in hello function, update to use f-string for return instead of concatenating strings with plus symbol 2. in POST example, fix dict reference to use key as dicts do not support dot notation by default
…per. Additionally updated publish to a queue example to use json.dumps() as opposed to the converter function and removed the unnecessary import
Howdy and thanks for contributing to our repo. The Cloudflare team reviews new, external PRs within two (2) weeks. If it's been two weeks or longer without any movement, please tag the PR Assignees in a comment. We review internal PRs within 1 week. If it's something urgent or has been sitting without a comment, start a thread in the Developer Docs space internally. PR Change SummaryUpdated Python worker examples to improve clarity and compatibility with Python 3.6+. Changed dot notation to key notation for JSON objects, replaced string concatenation with f-strings, and refined examples for better Pythonic practices.
Modified Files
How can I customize these reviews?Check out the Hyperlint AI Reviewer docs for more information on how to customize the review. If you just want to ignore it on this PR, you can add the Note specifically for link checks, we only check the first 30 links in a file and we cache the results for several hours (for instance, if you just added a page, you might experience this). Our recommendation is to add |
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.
Thank you for putting this together. Just one thing that I think needs to be changed, the rest looks good.
@dom96 I think it should actually be ok; looking at the docs for the worker binding here: https://developers.cloudflare.com/queues/configuration/javascript-apis/#queue
and the api docs here for the push method: https://developers.cloudflare.com/api/python/resources/queues/subresources/messages/methods/push/ I think it's okay - as long as it's structured data that can be cloned (and a string is in that category) we're okay. The larger issue might be if you have a js app that is reading from the queue and expecting a js object instead of a json string. Philosophically I'd say that for interop, you'd never want a real javascript object in your queue because you won't have a guarantee that whatever is reading the queue can interpret a javascript object. Pragmatically in this example nothing's actually reading from the queue Finally, the comment on the line above (95 before merge/96 after) says we're sending a json object to the queue. From what I can tell, to_js makes that actually incorrect, because to_js actually has us pushing/sending an actual javascript object. I'm okay with removing the updates, but if I do, we should probably update the comment on line 95 to be correct. Let me know your preference and I'll make the necessary adjustments if needed. |
@sjsadowski You're totally right. Thanks for looking into this in so much detail! |
@sjsadowski really great additions - thanks so much for the PR |
Summary
Confirmed with @dom96 that the examples should use the
@handler
wrapper.Python worker examples used dot notation for json objects converted to dicts, which doesn't work out of the box. In order to keep the examples simple, changed dot notation to key notation. Also removed string concatenation using plus signs and string.format() to both be f-strings as the current releases of pyodide all target support for python > 3.6
Updated the queue example to remove the unused import, and alter the example to be more pythonic with json.dumps()
Documentation checklist