Repository navigation
Expose duration through DispatchInfo - #121
shsms wants to merge 3 commits into
Conversation
Sorry, something went wrong.
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Some dispatches need the duration to make an execution plan. Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
| options: dict[str, Any] | ||
| """Additional options.""" | ||
|
|
||
| duration: timedelta | None | ||
| """The duration of the dispatch.""" | ||
|
|
There was a problem hiding this comment.
Just suggestion: duration can be in options dict.
However I don't know why you need this so feel free to ignore this comment.
Sorry, something went wrong.
llucax
left a comment
There was a problem hiding this comment.
Requesting changes to avoid accidental merges until we figure this out.
This solution will not work well in the general case.
Sorry, something went wrong.
| components=dispatch.target, | ||
| dry_run=dispatch.dry_run, | ||
| options=dispatch.payload, | ||
| duration=dispatch.duration, |
There was a problem hiding this comment.
This is tricky and can be misleading. If you use the high-level interface and you are listening to the start/stop events, or your actor is managed, this duration might not match the time your actor is really running if some dispatches were merged.
I think a better approach would be to include a list of active dispatches, and just pass the full Dispatch object there. Another approach would be to pass here a "merged duration" instead of the raw duration of the current dispatch. Depending on the use case one could be more useful than the other.
Sorry, something went wrong.
There was a problem hiding this comment.
I moved the conversation to #119, I think it is more appropriate to discuss the design in the issue.
Sorry, something went wrong.
No description provided.