Merged
Conversation
|
|
cretz
reviewed
Apr 30, 2025
Member
cretz
left a comment
cretz
left a comment
There was a problem hiding this comment.
Thanks! Minor suggestion, then happy to merge
Comment on lines
43
to
44
| # unreachable | ||
| return "" |
Member
There was a problem hiding this comment.
Suggested change
| # unreachable | |
| return "" |
continue_as_new return type hint has https://docs.python.org/3/library/typing.html#typing.NoReturn, so shouldn't need this part (but didn't test, maybe mypy needs it)
Contributor
Author
There was a problem hiding this comment.
I've made the changes. Let's have the CI workflows check it.
l33oo
force-pushed
the
fix/periodic_polling
branch
from
10fdec6 to
88b3a88
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was changed
Fixed the retry loop in the "Periodic Polling of a Sequence of Activities" example.
Reference taken from the
samples-javaimplementation.Why?
Previously,
continue_as_newwas inside the retry loop, causing the workflow to restart after just one failed attempt, instead of retrying up to 10 times as intended.Checklist
Closes [Bug] continue_as_new prematurely exits retry loop in "Periodic Polling of a Sequence of Activities" example #182
How was this tested:
Manually tested by running the workflow and verifying that it retries 10 times before continuing as new. Also validated expected logging behavior.
Any docs updates needed?
No updates needed.