Add an archival task executor #3663
Conversation
MichaelSnowden
force-pushed
the
da-q-processors
branch
2 times, most recently
from
1727ed9
to
2a28663
Compare
|
The title should be archival task executor? |
MichaelSnowden
changed the title
MichaelSnowden
force-pushed
the
da-q-processors
branch
2 times, most recently
from
9bc7a96
to
0457a6d
Compare
MichaelSnowden
force-pushed
the
da-q-processors
branch
6 times, most recently
from
124f601
to
9ee1de8
Compare
|
MichaelSnowden
force-pushed
the
da-q-processors
branch
from
9ee1de8
to
84a8be9
Compare
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if mutableState == nil { |
There was a problem hiding this comment.
Could this happen if a workflow is deleted before the archival task?
There was a problem hiding this comment.
Changed to drop the task and log a warning. Also added a test case for this branch.
There was a problem hiding this comment.
return nil error to drop the task?
There was a problem hiding this comment.
I return a const error here and then I convert it to nil further up the stack. If I return nil here, then execution will continue because this is a helper method.
MichaelSnowden
force-pushed
the
da-q-processors
branch
2 times, most recently
from
869d8ed
to
604cc96
Compare
| logger := log.With(e.logger, tag.Task(task)) | ||
| request, err := e.getArchiveTaskRequest(ctx, logger, task) | ||
| if err != nil { | ||
| if err == ErrMutableStateIsNil || err == ErrWorkflowExecutionIsStillRunning { |
There was a problem hiding this comment.
nit: either handle at the top level in Execute(...) or from the return function loadAndVersionCheckMutableState
There was a problem hiding this comment.
Good idea, thanks!
MichaelSnowden
force-pushed
the
da-q-processors
branch
from
604cc96
to
7e0a1b3
Compare
What changed?
Added a processor which executes tasks on the archival queue. A followup PR will actually enable the queue factory.
Why?
I made these changes to enable the archival queue.
How did you test it?
I tested this using unit tests because it's not actually hooked into prod yet.
Potential risks
No--it's not wired into main yet.
Is hotfix candidate?
No.