-
Notifications
You must be signed in to change notification settings - Fork 57
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
Refactor: Correct polling infrequent/periodic_sequence usecase #125
base: main
Are you sure you want to change the base?
Conversation
|
polling/infrequent/activities.py
Outdated
@@ -13,7 +13,8 @@ class ComposeGreetingInput: | |||
|
|||
@activity.defn | |||
async def compose_greeting(input: ComposeGreetingInput) -> str: | |||
test_service = TestService() | |||
attempt = activity.info().attempt - 1 | |||
test_service = TestService(attempt=attempt) |
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! Seems this was written with a bug.
I think a cleaner approach would be to make this activity a method (and call via execute_activity_method
from workflow) and instantiate the TestService
as class state.
Also, I think frequent
polling is affected by this same test service issue.
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 good, thanks! May need reformat (poe format
) and may need to check types (poe lint-types
). If you want you can also apply to the other polling test that uses this test service, but no worries if not, we can create another issues.
|
What was changed
We use the activity.info().attempt value to feed the TestService
Why?
Before this change the attempt value in the TestService was never change and stay to 0
Checklist
Run the 2 command explain in the readme - success on 5 atempts