IWF-163: Add retry policy to QueryWorkflow calls #491
Conversation
lwolczynski
force-pushed
the
jira/lwolczynski/IWF-163
branch
from
68fa7a7
to
7feffc2
Compare
| retryPolicy := temporalapi.QueryWorkflowFailedRetryPolicy{ | ||
| InitialIntervalSeconds: config.GetInitialIntervalSecondsWithDefault(), | ||
| MaximumAttempts: config.GetMaximumAttemptsWithDefault(), | ||
| } | ||
| unifiedClient = temporalapi.NewTemporalClient(temporalClient, config.Interpreter.Temporal.Namespace, converter.GetDefaultDataConverter(), false, retryPolicy) |
There was a problem hiding this comment.
If you have config.GetInitialIntervalSecondsWithDefault(), then the expectation is that the code that need to use the value will call it to get the value with default, instead of passing into here.
So it will be temporalapi.NewTemporalClient(temporalClient, config.Interpreter.Temporal.Namespace, converter.GetDefaultDataConverter(), false, config.ApiConfig.QueryRetryPolicy)
Also consider using pointers to make this optional and load the default values in client.go.
Basically the config can be defined it as a pointer instead of struct. So that internally, we just need to pass a nil into this, which should let it use the default value.
| qres, err = queryWorkflowWithStrongConsistency(t, ctx, workflowID, runID, queryType, args) | ||
| if err != nil { | ||
| if t.isQueryFailedError(err) { | ||
| if attempt == t.queryWorkflowFailedRetryPolicy.MaximumAttempts { |
There was a problem hiding this comment.
Nit: I don't think we need another check here, since it'a already checked by the for loop condition?
Although we will have to check err != nil again outside of the loop
Description
Checklist
Related Issue
Closes #437