-
Notifications
You must be signed in to change notification settings - Fork 56
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
adds test example for hello_update.py #118
Conversation
|
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.
Thanks! Just a couple of minor things
(also note Python samples CI is broken, we are fixing in #119 now) |
This test looks good, thanks! But I think CI is failing for a known update bug fix we haven't released yet. In temporalio/sdk-python#521 we made sure that workflows and updates completed in the same task are properly handled. You may be able to update to Python SDK 1.6.1 to check (update |
No worries, just thought of adding the test while I was exploring the update feature and did not find a test for it. Can wait 👍 |
Co-authored-by: Chad Retz <[email protected]>
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.
Make sure you reformat (e.g. poe format
), CI is currently failing
Yep, my bad - I had just accepted the suggestion you shared - fixed now |
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.
Looks great, thanks!
What was changed
adds test example for hello_update.py
Why?
Checklist
Closes
How was this tested: