feat: translate TaskRunner #1
Conversation
therealvio
force-pushed
the
feat/translate-task-runner
branch
from
90ed9ea
to
74fd31c
Compare
therealvio
force-pushed
the
feat/translate-task-runner
branch
2 times, most recently
from
9e0d7ac
to
4154d59
Compare
There was a problem hiding this comment.
In general this looks good. Let's align the versions of Go we are using, though.
Additionally, I think it's worth considering fuzzing for our tests in order to give us some additional confidence. We can assume some structure around our inputs given the requirements imposed by AWS.
therealvio
force-pushed
the
feat/translate-task-runner
branch
from
4154d59
to
fa38f07
Compare
Is this something we want to maybe include in the related jira ticket as a checklist item so we can break it off into a dedicated card? |
therealvio
force-pushed
the
feat/translate-task-runner
branch
3 times, most recently
from
aa6397b
to
ad783ba
Compare
src/plugin/task-runner.go
Outdated
| waiterClient := ecs.NewTasksStoppedWaiter(ecsClient, func(o *ecs.TasksStoppedWaiterOptions) { | ||
| o.MinDelay = time.Second | ||
| o.MaxDelay = 10 * time.Second | ||
| }) | ||
| result, err := awsinternal.WaitForCompletion(ctx, waiterClient, taskArn) | ||
| if err != nil { | ||
| // TODO: Do we wanna wanna go from fatal, to print, and provide an opportunity to share logs from the task if there any? | ||
| // That, or we include the log sharing logic within this condition | ||
| log.Printf("error waiting for task completion: %v", err) | ||
| log.Fatalf("failure information: %v", result.Failures[0]) | ||
| } |
There was a problem hiding this comment.
In the murmur implementation, this was done by checking the result of the waiter. Our waiter doesn't include this information. Instead, we're working with the Golang SDK by taking the error and result values and using them in the error handling, rather than waiting near the end of the execution (like in the original implementation).
We're failing fast, and that's probably a better pattern than carrying on needlessly.
therealvio
force-pushed
the
feat/translate-task-runner
branch
3 times, most recently
from
e20d05e
to
8600d08
Compare
therealvio
force-pushed
the
feat/translate-task-runner
branch
from
c7139c5
to
e97bc67
Compare
therealvio
force-pushed
the
feat/translate-task-runner
branch
from
e97bc67
to
3647c91
Compare
therealvio
force-pushed
the
feat/translate-task-runner
branch
2 times, most recently
from
54f21b5
to
45ba9f1
Compare
therealvio
force-pushed
the
feat/translate-task-runner
branch
2 times, most recently
from
6fb7826
to
5c9d3bd
Compare
This change is a naive golang translation of the `TaskRunner` code from Murmur (refer to notes). The aim of this commit is to get the initial code into the repo and start building on it from there. Notes: - TaskRunner permalink: https://github.com/cultureamp/murmur/blob/260f8936d59aaa6074c6fc6f31b935e011bda9f6/ops/migrations-runner/lib/TaskRunner.ts - migrations-runner permalink: https://github.com/cultureamp/murmur/tree/078d0154b1215f226df5d3d081dfd131a8ae898a/ops/migrations-runner
The value to have deterministic import orders in our code doesn't seem that useful considering that imports will automatically be cleaned up and sorted thanks to the native tooling. This should be good enough without another opinionated tool complaining that the order isn't _that_ specific way. It's low-impact, and not worth having 2 toolings fight over this.
therealvio
force-pushed
the
feat/translate-task-runner
branch
from
5c9d3bd
to
ba7806d
Compare
|
Purpose 🎯
These changes transfer the original
TaskRunnercode from one of our services into this repo. The selected service represented our "favoured" application of the TaskRunner pattern that we want to decompose into a Buildkite plugin. This involved translating from Typescript to Golang. This change will act as the starting point for further iteration as we move towards making it how we need it to be.Context 🧠
ecs-task-runnerBuildkite plugin CSRE-4944