Skip to content
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

Change minimum Python to 3.9 and add Trio sample #162

Merged
merged 2 commits into from
Feb 5, 2025

Conversation

cretz
Copy link
Member

@cretz cretz commented Feb 5, 2025

What was changed

  • Changed minimum Python version to 3.9
    • We'd have to do this anyways when we upgrade to next Python release, just didn't do the 3.13 update in CI
    • I'll update the required CI checks to be 3.9 just before merge of this PR
  • Added sample for Trio demonstrating using Temporal in a codebases that have fully adopted Trio

Checklist

  1. Closes Sample request: Trio #161

@cretz cretz requested a review from a team February 5, 2025 19:00
Copy link
Contributor

@dandavison dandavison left a comment

Choose a reason for hiding this comment

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

Nice, a few comments aimed at making the comments maximally understandable.

from trio_async import workflows


@trio_asyncio.aio_as_trio # Note this decorator which allows asyncio primitives
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you expand this comment slightly, are we basically saying

without this decorator you wouldn't be able to use anything from the asyncio module.

?

Copy link
Member Author

@cretz cretz Feb 5, 2025

Choose a reason for hiding this comment

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

Yes, "allows asyncio primitives" was meant to imply "needed to allow asyncio primitives", otherwise if it wasn't needed we could remove the decorator. I can rephrase if needed (though don't want to effectively re-document https://trio-asyncio.readthedocs.io/en/latest/usage.html#calling-asyncio-from-trio). I admit I didn't go over all of asyncio module to confirm you can use nothing without this, so not sure the statement about "anything" is correct.

Comment on lines +14 to +15
# custom event loop. Therefore Trio primitives should never be used in a
# workflow, only asyncio helpers (which delegate to the custom loop).
Copy link
Contributor

Choose a reason for hiding this comment

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

These comments are very helpful. Two teaks: if "primitives" and "helpers" are the same thing can we use the same word, and can you give an example so that it's really clear what we can and can't do. Something like this:

Suggested change
# custom event loop. Therefore Trio primitives should never be used in a
# workflow, only asyncio helpers (which delegate to the custom loop).
# custom event loop. Therefore Trio primitives such as XXX should never be used in a
# workflow, only functions and classes from the asyncio module, (which delegate to the custom loop).

Copy link
Member Author

@cretz cretz Feb 5, 2025

Choose a reason for hiding this comment

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

I intentionally did helpers here because to me primitives is helpers + event loop, and here it's just helpers. I want the user to understand that it's only the high-level helpers of asyncio used in the workflow, not asyncio itself.

from trio_async import activities, workflows


@trio_asyncio.aio_as_trio # Note this decorator which allows asyncio primitives
Copy link
Contributor

Choose a reason for hiding this comment

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

As above, would be good to expand and clarify this. Are we saying

without this decorator you wouldn't be able to use anything from the asyncio module.

?

Copy link
Member Author

@cretz cretz Feb 5, 2025

Choose a reason for hiding this comment

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

Yes, I can adjust. I do not believe it is true that it's anything from the asyncio module because I didn't go over everything in that module. Is there another way to phrase it? Or is what's there ok?

@cretz
Copy link
Member Author

cretz commented Feb 5, 2025

Assuming the other wording is good enough, merging. Can revisit if needed.

@cretz cretz merged commit 81b5098 into temporalio:main Feb 5, 2025
10 checks passed
@cretz cretz deleted the trio-sample branch February 5, 2025 23:14
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.

Sample request: Trio
2 participants