feat: add blueapi_task_id to metadata - #1628
Conversation
f914649 to
98f442e
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1628 +/- ##
=======================================
Coverage 95.96% 95.96%
=======================================
Files 45 45
Lines 3317 3318 +1
=======================================
+ Hits 3183 3184 +1
Misses 134 134 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
98f442e to
8b57ff0
Compare
| """ | ||
| task.prepare_params(self._ctx) # Will raise if parameters are invalid | ||
| task_id: str = str(uuid.uuid4()) | ||
| if self._ctx.tiled_conf is not None: |
There was a problem hiding this comment.
There 2 questions that came to my mind
- I don't know if it is going to be useful to have blueapi_task_id regardless if you are using tiled.
- The code can be just
task.metadata["blueapi_task_id"] = task_id #type: ignore
I have done the dance only to not put the type ignore
There was a problem hiding this comment.
I think we should keep the metadata the same regardless of whether tiled is being used.
I wouldn't be against making the field dict. There's no real benefit to keeping it Mapping when it's only our code using it and we're always using dicts.
Was the prepare_params intentionally moved below the task id handling? Might be better to keep failing fast if the parameters are wrong.
There was a problem hiding this comment.
I'm in agreement that one extra bit of meta won't be an issue and it keeps all the data align in a way.
8b57ff0 to
1bc670c
Compare
3 participants
closes #1554