[SFN] Base Support for States.DataLimitExceeded #10511
Conversation
MEPalma
added
the
semver: patch
|
|
||
|
|
||
| def is_within_size_quota(value: Union[str, json]) -> bool: | ||
| item_str = value if isinstance(value, str) else to_json_str(value) |
There was a problem hiding this comment.
StepFunctions is tricky with all these conditional castings 😅
| "End": true | ||
| } | ||
| } | ||
| } |
| "Next": "ProcessResult", | ||
| "Catch": [ | ||
| { | ||
| // Note: DataLimitExceeded is a terminal error that can't be caught by the States.ALL error type. |
There was a problem hiding this comment.
Putting json5 to good use with a useful comment 😃
| create_lambda_function( | ||
| func_name=function_name, | ||
| handler_file=EHT.LAMBDA_FUNC_LARGE_OUTPUT_STRING, | ||
| runtime="python3.9", |
There was a problem hiding this comment.
Can we use the typed Runtime.python3_12 here instead?
It would be great to adopt the latest Lambda runtimes for new tests because the al2023 image size is massively smaller, saving us pull time.
There was a problem hiding this comment.
same for the other new test cases
FYI: Bojan is currently updating the Lambda runtimes for existing tests, so no need to update them to prevent conflicts
| create_state_machine, | ||
| sfn_snapshot, | ||
| ): | ||
| four_bytes_utf8_char = "𐍈" |
| "$..tracingConfiguration", | ||
| ] | ||
| ) | ||
| class TestStatesErrors: |
There was a problem hiding this comment.
Thorough coverage of error scenarios 👏
💡 A short high-level overview of the key test cases (e.g., as class docstring) could improve clarity. That would clarify the two cases service_task_lambda and task_lambda.
There was a problem hiding this comment.
More of a thought input, rather than actionable:
🧠 Given the inherent complexity of StepFunctions, it's impossible to test all possible scenarios. I wonder whether you have some intuition for a testing strategy to provide good enough coverage while keeping an eye on the test execution time?
| def exec_lambda_function(env: Environment, parameters: dict, region: str, account: str) -> None: | ||
| lambda_client = boto_client_for(region=region, account=account, service="lambda") | ||
|
|
||
| invocation_resp: InvocationResponse = lambda_client.invoke(**parameters) | ||
|
|
||
| func_error: Optional[str] = invocation_resp.get("FunctionError") | ||
| payload_json = json.load(invocation_resp["Payload"]) |
There was a problem hiding this comment.
ups, did I break something with this refactoring?
That's a cumbersome decoding step, do we have a regression test to catch that?
There was a problem hiding this comment.
maybe a clarifying comment on why this extra step is needed could help. Is this payload extraction handling the decoding error necessary in other places of LocalStack as well 🤔 ?
There was a problem hiding this comment.
Thank you! I have added some in-code comments to explain the goal of the _from_payload function. The lambda tests added by this pr will control the case in which the output of the lambda is a to be handled by SFN as a string and not a json string. I will also add a test for string payloads in lambda suite in the services package test_invoke_string_payload.
|
LS style: We usually write something like |
Motivation
Currently the States error States.DataLimitExceeded is not supported by the StepFunctions v2 interpreter. This PR adds grammar support for such States error name, and base support for raising States.DataLimitExceeded exceptions whenever the size of a state or task response is above the quota.
Addresses: #10489
Changes