Add transaction mode to worker

[callumforrester]

Draft PR with a suggested transaction mode for the worker, transaction mode enables creating of new tasks before they are submitted.

To consider:

• Is the transaction API right?
• Should we allow multiple pending transactions or only one at a time? Currently we lock the transaction mode if one is in progress.
• Can the implementation be simplified? (probably yes)
• Should we still allow non-transaction plan running? (probably not, as it provides a way around the aforementioned transaction lockout)

[codecov]

Codecov Report

Merging #202 (f8b9342) into main (febaf8e) will increase coverage by 0.24%.
The diff coverage is 89.28%.

❗ Current head f8b9342 differs from pull request most recent head ebb94e8. Consider uploading reports for the commit ebb94e8 to get more accurate results

@@            Coverage Diff             @@
##             main     #202      +/-   ##
==========================================
+ Coverage   89.58%   89.83%   +0.24%     
==========================================
  Files          40       40              
  Lines        1143     1151       +8     
==========================================
+ Hits         1024     1034      +10     
+ Misses        119      117       -2     

Impacted Files	Coverage Δ
src/blueapi/cli/amq.py	38.88% <0.00%> (ø)
src/blueapi/worker/task.py	100.00% <ø> (ø)
src/blueapi/cli/updates.py	40.00% <25.00%> (ø)
src/blueapi/worker/reworker.py	93.71% <93.10%> (+2.80%)	⬆️
src/blueapi/service/main.py	80.48% <100.00%> (-0.91%)	⬇️
src/blueapi/service/model.py	100.00% <100.00%> (ø)
src/blueapi/worker/__init__.py	100.00% <100.00%> (ø)
src/blueapi/worker/event.py	96.00% <100.00%> (-2.00%)	⬇️
src/blueapi/worker/worker.py	100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[tpoliaw]

• Is the transaction API right?
  The general approach of, "submit task -> get ID", "Start task by ID" is ok. I think the method names should reflect the domain rather than the pattern they're following. submit_task(t: Task) -> TaskID: , run_task(id: TaskID) -> Status, drop_task(id: TaskID) -> Status, pending_tasks() -> list[TaskID]. Should possibly be some way of querying what a pending task is as well to get the original Task back.
  It would be good if submitting tasks also validated them so users/clients get feedback that the task they're running is valid or not. A task being accepted but invalid doesn't make much sense.
• Should we allow multiple pending transactions or only one at a time?
  Multiple probably makes most sense, otherwise you could be blocked by a pending but ignored request. If each submitted task has a timestamp, there could be some expiration that prevents the pending tasks piling up.
• Can the implementation be simplified?
  Always but it's a good starting point to build on. It might end up needing to be refactored anyway to allow handling of the expiration/validation to be added.
• Should we still allow non-transaction plan running?
  No

[callumforrester]

@tpoliaw API design makes sense, have implemented. The validation on submission sounds very sensible and has also been identified as a requirement for Artemis. I think that should go in a separate PR to avoid bloating this one.

More stream-of-conciousness stuff: Generally agree with everything else although I'm a bit concerned about the expiry time solution, since it may be indeterministic and hard to test. Also, is it even blueapi's job to manage the state of everyone's potential tasks? One thought that did occur to me would be to have a stack of tasks, which would be useful for things like pausing/resuming but I haven't thought it out properly yet. Any thoughts welcome.

[tpoliaw]

For the expiration side, would an OrderedDict[id, (task, timestamp)] work? Each task that's submitted would be added to the dict. If run_task is called for a task that is older than x seconds, it fails. If there are more than y tasks in the queue when a new one is added, remove any expired ones. As the dict is ordered, this would be a single check in the usual case. It would be deterministic but the x/y parameters would be kept as an implementation detail and only the timestamp is actually part of the API. It could be returned with the taskID?

If only one task is running at once, a stack probably isn't needed although the current task would need to be available somewhere.

[callumforrester]

Cheers, all good spots. I'm a bit more convinced by the expiry time argument now, but want to do it as a separate PR, possibly along with the REST stuff, to avoid bloating this one any further.
I'm also considering deferring it since we can reasonably expect our own client implementation in GDA to do its own bookkeeping. Maybe we should wait until we have other clients that don't?

[rosesyrett]

Looks clean to me