Skip to content
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

aws-step-functions-tasks: Wrong property declared on TaskBaseState props #25931

Closed
1 of 2 tasks
protyay opened this issue Jun 11, 2023 · 5 comments
Closed
1 of 2 tasks
Labels
@aws-cdk/aws-stepfunctions-tasks closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. feature-request A feature should be added or improved. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.

Comments

@protyay
Copy link

protyay commented Jun 11, 2023

Describe the feature

The timeout property in the LambdaInvokeProps interface is inherited from the TaskStateBaseProps interface.
This timeout refers to the overall timeout of the state machine.

Why should each state configuration inherit this property ? This should be defined during the construction of the StateMachine instance.

Use Case

Removing the property of timeout from TaskBaseState props would reduce confusion

Proposed Solution

Remove the timeout property from the TaskStateBaseProps .
Overall, timeout property already exists and should be used on the StateMachine construct

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.83

Environment details (OS name and version, etc.)

macOS Venture 13.4

@protyay protyay added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jun 11, 2023
@peterwoodworth
Copy link
Contributor

timeout is deprecated on TaskBaseStateProps, however totalTimeout is available and sets the timeout for the task itself. I'm not sure I understand the exact request here

@peterwoodworth peterwoodworth added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels Jun 12, 2023
@github-actions
Copy link

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Jun 14, 2023
@protyay
Copy link
Author

protyay commented Jun 16, 2023

@peterwoodworth – I see, I have probably not seen the deprecated tag on the timeout field.
I think you meant taskTimeout , because I was not able to find totalTimeout on the TaskStateBaseProps interface.

Also, I have a different question, if the LambdaInvoke task has defined a timeout and also a timeout has been defined during the Lambda definition, then the one which is lesser would take preference, right ?

I'm not sure I understand the exact request here

I think I was referring to a different timeout property, probably from v1. Sorry for the confusion.

@github-actions github-actions bot removed closing-soon This issue will automatically close in 4 days unless further comments are made. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Jun 16, 2023
@peterwoodworth
Copy link
Contributor

I think you meant taskTimeout , because I was not able to find totalTimeout on the TaskStateBaseProps interface.

Whoops! you're right, my bad 😄

Also, I have a different question, if the LambdaInvoke task has defined a timeout and also a timeout has been defined during the Lambda definition, then the one which is lesser would take preference, right ?

I'm not positive, but I think this would effectively be the case. There might be a very slight difference between the two, depending on if the task takes a slight amount of the timeout window to invoke the function and receive the result or not

@peterwoodworth peterwoodworth added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Jun 16, 2023
@github-actions
Copy link

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Jun 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-stepfunctions-tasks closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. feature-request A feature should be added or improved. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.
Projects
None yet
Development

No branches or pull requests

2 participants