-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Set proper timeout for canary workflows #398
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
Set proper timeout for canary workflows #398
Conversation
| wfTypeReset, | ||
| wfTypeHistoryArchival, | ||
| wfTypeVisibilityArchival, | ||
| //wfTypeHistoryArchival, |
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.
why are you commenting these out? These can be excluded through config.
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.
They are not registered and generate errors. Should I register them instead?
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.
FWIW I see errors from wfTypeVisibilityArchival even after I've added it to the excludes list.
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.
I now understand that these will fail if the cluster does not have archiving enabled 😄
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.
Yes register them instead of commenting it out. There is already config based mechanism to exclude them.
|
I patched this change into my v0.23.0 canary and it seems to have solved the problem 😄 |
|
Would it be possible to get this merged for the next release? 😄 |
What changed?
WorkflowRunTimeoutshould be set instead ofWorkflowExecutionTimeoutin canary.Why?
This PR closes #394.
How did you test it?
Run canary locally for 1 hour and check workflow state with:
Potential risks
No risks.