[SFN] Add support for Task Timeouts #8376
Conversation
tests/integration/stepfunctions/v2/callback/test_callback.snapshot.json
Outdated
Show resolved
Hide resolved
| "Cause": "", | ||
| "error": { | ||
| "Error": "States.Timeout", | ||
| "Cause": "" |
There was a problem hiding this comment.
Did you observe any case where this wasn't just empty or is this a static thing?
There was a problem hiding this comment.
I did try to generate some cause message but was never able for "States.Timeout" errors.
...k/services/stepfunctions/asl/component/state/state_execution/state_task/lambda_eval_utils.py
Show resolved
Hide resolved
localstack/services/stepfunctions/asl/component/state/state_execution/state_task/state_task.py
Show resolved
Hide resolved
| "States": { | ||
| "Start": { | ||
| "Type": "Task", | ||
| "TimeoutSecondsPath": "$.TimeoutSecondsValue", |
There was a problem hiding this comment.
Please correct me if I'm wrong but I think I didn't see a test that has both a TimeoutSeconds as well as TimeoutSecondsPath field set. Might be good to add 🤔
There was a problem hiding this comment.
You're right, there are no tests with both declared. This is because it is part of the linter logic which we do not support yet (for example: https://docs.aws.amazon.com/step-functions/latest/dg/amazon-states-language-task-state.html#task-state-fields)
| thread = Thread(target=_exec_and_notify) | ||
| thread.start() | ||
| finished_on_time: bool = terminated_event.wait(timeout_seconds) |
There was a problem hiding this comment.
Need to be a bit more careful here regarding potential leaks.
It's not the nicest patterns but there's the FuncThread and/or TMP_THREADS concepts to keep track of started threads so that we can kill them on shutdown.
Let's keep it as is for now though and revisit the concurrency constructs in a separate project at a later stage 👍
Add support for Task
TimeoutSecondsandTimeoutSecondsPathand relevant test cases.