OpenFn / OpenFn/lightning

Maybe remove snapshot_id from workorders

Open
#4,241 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

question
Dominant language
Elixir
Stars
296
Forks
86
Avg merge
1d 13h
Merged PRs (30d)
50

Description

Following on #4186 , we might want to remove snapshot_id from workorders. The big question is this: Why is it there?

The Docs

Work Orders (Key Concept): https://docs.openfn.org/documentation/get-started/terminology#work-order
Work Order Status: https://docs.openfn.org/documentation/monitor-history/status-codes#work-order-status

The canonical OpenFn user experience is this

  1. create "generate patient ID" workflow (wf1:v1)
  2. receive work order (wo1) to "generate patient ID" for "Stu"
  3. run (r1) for wo1 fails because of a syntax error in step 3
  4. fix the syntax error (now we are on wf1:v2)
  5. run (r2) for wo1 succeeds
  6. receive work order (wo2) to "generate patient ID" for "Aleksa"
  7. run (r3) for wo2 succeeds
Now you have:
  • 1 workflow (with 2 versions),
  • 2 work orders in a "success" state (you have generated patient IDs for 2 citizens),
  • and you have 3 runs (2 succeeded, 1 failed).

What's the rationale for having a snapshot_id on the work_order? If it's some sort of denormalization/performance thing, maybe we rename it to snapshot_for_initial_run to disambiguate?

At one time, it might have been necessary to ensure that the first run is executed with the appropriate snapshot. This makes good sense to me. If the work order is created at 7pm, when the workflow is on version 1 and it's number 423,973 in the queue, when the first run is finally created at 9pm, the workflow is now on version 2. We want to ensure that the initial run for the WO is executed with the version that was in place when the order to do the work (i.e., workorder) was created.

But @lmac-1 points out that the Work Order and its first Run are created synchronously in the same database transaction:

lib/lightning/work_orders.ex:97-120
def create_for(%Trigger{} = trigger, attrs, opts \\ []) do
  Multi.new()
  |> Multi.insert(:dataclip, fn _ -> ... end)
  |> Multi.insert(:workorder, fn %{dataclip: dataclip} ->
      build_for(trigger, attrs)  # ← Creates BOTH WO and Run together
    end)
  |> Repo.transaction()
end

lib/lightning/work_orders.ex:201-269
def build_for(%Trigger{} = trigger, attrs) do
  snapshot = Snapshot.get_current_for(attrs[:workflow])  # ← Fetched once
  
  build(attrs)
  |> put_assoc(:snapshot, snapshot)      # ← Set on WorkOrder
  |> put_assoc(:runs, [
      Run.for(trigger, %{
        snapshot: snapshot,                # ← Same snapshot used for Run
        # ...
      })
    ])
end

The snapshot is fetched once and used for both the Work Order and Run atomically. I don't think that there is a time window where the workflow could be updated between WorkOrder creation and Run creation. The "queue" that matters is the Run sitting in the worker queue waiting to execute, not a delay between WorkOrder and Run creation.

If this understanding is correct, wouldn't Run.snapshot_id alone preserve queue integrity? The Run already has its snapshot_id set when it enters the worker queue. Is there a scenario where Work order and Run creation could be separated by time?

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading lib/lightning/work_orders.ex:97-120 and 201-269, then trace how the WorkOrder and Run snapshots are created and used. Compare that behavior with the Work Order and Work Order Status documentation linked in the issue, and establish whether removing or renaming snapshot_id preserves the documented behavior; done means a resolved design decision with the affected references identified.

Written by the indexing model from the issue text.

Assessment

Tech stack
elixir
Domain
backend, database
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.