-
Notifications
You must be signed in to change notification settings - Fork 61
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
- create "generate patient ID" workflow (wf1:v1)
- receive work order (wo1) to "generate patient ID" for "Stu"
- run (r1) for wo1 fails because of a syntax error in step 3
- fix the syntax error (now we are on wf1:v2)
- run (r2) for wo1 succeeds
- receive work order (wo2) to "generate patient ID" for "Aleksa"
- 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
# ...
})
])
endThe 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?
Metadata
Metadata
Assignees
Labels
Type
Projects
Status