Remove final_dataclip_id

Overview

When the Worker finishes executing a workflow, it sends a run:complete event to Lightning with a final_dataclip_id property. Which, naturally, is the string id of the dataclip (the state) for the last step in the workflow.

It turns out that this final_dataclip_id isn't actually used by Lightning.

We can safely remove the key from the worker's payload without affecting Lightning, this is trivial. The catch is that a lot of the worker's unit tests rely on final_dataclip_id to assert on the result of a workflow.

The task is to go though the failed tests and update them with an alternative means of loading final state.

Requirements

Remove the final_dataclip_id key from the run:complete payload
Update unit tests

Background

The Worker is direct peer dependency of Lightning. When users trigger Workflows in Lightning, they are "claimed" by the worker (which is a separate service running on a different IP) and processed.

While the Worker executes a workflow, it sends back events to Lightning via websocket.

See the worker readme in packages/ws-worker/README.md for a bit of an overview

The actual execution is handled by the engine (engine-multi). The Worker package often mocks out the engine in tests so that it can focus on the Lighting-Worker communication.

Implementation Notes

The fix here should barely touch files in src/, except to delete a line or two. The bulk of the diff should be in test files.

Tests in different files will need different solutions - I doubt there's a single solution which can be blanket applied to all failing tests.

There are several alternative means of getting the final result of a workflow, and you'll find these patterns used throughout the test code.

The lightning mock, used extensively in unit tests, has APIs including getResult(runId) and waitForResult(runId), which are a convenient means to get the final dataclip
Listen to step:complete instead of run:complete, because step complete also includes a dataclip id. In most unit tests there is only one step so this is an easy change. In some tests, you'll have to listen to the correct step

The trick with this is that lightning doesn't use this - but my test framework does!

I have a bunch of tests around the idea of run an attempt, get an output back. that output relies on final data clip id id.

I need to make changes to work without this key. Options:

Re-write tests to be focused around runs, rather than attempts, because an attempt doesn't really have a result
Have the mock return the result states of each run (probably by job id), and have the tests use that
Make the "result" of an attempt the output of the final run (which is what I do now). But this drifts quite far from Lightning so I'm a bit uncomfortable.

I think I'm gonna have to bite the bullet here and replace my mock getResult with getRunResults and getJobResults. Ok, the job abstraction is less interesting to lightning, but it is useful for us because it has predictable id and I can test on it.

So the effort here is around unit tests.

OpenFn / kit