-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
[SFN] base implementation of intrinsic functions #7526
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…est suite updates, lintings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
A few remarks:
-
We can probably remove the number of files a bit by aggregating them which would be a bit more pythonic in general. E.g. Keep all the different function_argument_x classes in
function_argument.py
. This generally also makes reviewing, code navigation and refactoring in general a bit easier -
Do you think we should also verify the input argument types more? I'm not sure how stepfunction behaves when you for example try to add or multiply a number + string or when you have an array of non-string values. 🤔
@@ -0,0 +1,27 @@ | |||
from enum import Enum |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
file name has a typo (fuinction => function)
@@ -404,10 +409,10 @@ def check_invocations(): | |||
# clean up | |||
cleanup(sm_arn, state_machines_before, stepfunctions_client) | |||
|
|||
@pytest.mark.skip("Intrinsic Functions not yet supported.") | |||
# @pytest.mark.skip("Intrinsic Functions not yet supported.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should probably just be deleted right?
if os.environ.get("AWS_DEFAULT_REGION") != "us-east-1": | ||
pytest.skip("skipping non us-east-1 temporarily") | ||
# if os.environ.get("AWS_DEFAULT_REGION") != "us-east-1": | ||
# pytest.skip("skipping non us-east-1 temporarily") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
Adds base: lexer, grammar, components, and implementation of stepfunctions' intrinsic functions.
Unrelated test failure:
tests/integration/stepfunctions/legacy/test_stepfunctions_legacy.py::test_aws_sdk_task_delete_s3_object