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

[SFN] Fix optimised integrations clients leading to timeouts and retries #9732

Merged
merged 1 commit into from Nov 27, 2023

Conversation

MEPalma
Copy link
Contributor

@MEPalma MEPalma commented Nov 26, 2023

Motivation

Invocations of service integrations in SFN v2 currently trigger the default behaviour of boto for connection and read timeouts, as well as the retry strategy. This could potentially disrupt the State Machine's behaviour related to timeout and retry declarations.
Closes #9731

Changes

This pull request deactivates the mentioned boto behaviour to guarantee that timeout issues are passed upstream to the interpreter as intended.

@MEPalma MEPalma added the semver: patch Non-breaking changes which can be included in patch releases label Nov 26, 2023
@MEPalma MEPalma added this to the 3.1 milestone Nov 26, 2023
@MEPalma MEPalma self-assigned this Nov 26, 2023
@coveralls
Copy link

Coverage Status

coverage: 84.049% (+0.005%) from 84.044%
when pulling 84926df on MEP-sfn-fix_integrations_timeoutput
into 61d66a9 on master.

Copy link

LocalStack Community integration with Pro

       2 files  ±0         2 suites  ±0   1h 6m 50s ⏱️ - 2m 26s
2 358 tests ±0  2 043 ✔️ ±0  315 💤 ±0  0 ±0 
2 359 runs  ±0  2 043 ✔️ ±0  316 💤 ±0  0 ±0 

Results for commit 84926df. ± Comparison against base commit 61d66a9.

Copy link
Member

@dominikschubert dominikschubert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, seems to be a sensible timeout 👍

@noseworthy
Copy link

Many thanks @MEPalma and @dominikschubert. Localstack is a crazy ambitious project, and releasing a rewrite of stepfunctions (of all things) is super ambitious again. Thanks for being so responsive and getting these fixes out so fast. I see 3 SFN PRs up and approved this morning, well done 👏 . Hopefully all the regressions are caught soon and you're back to normal operating procedures soon! We're loving using localstack, keep up the great work!

@MEPalma MEPalma merged commit 1e4aa80 into master Nov 27, 2023
29 checks passed
@MEPalma MEPalma deleted the MEP-sfn-fix_integrations_timeoutput branch November 27, 2023 16:06
@noseworthy
Copy link

Hey @MEPalma and/or @dominikschubert : Any idea when this change will make it to the localstack-pro docker image? Do nightlies get built for that image as well? I see the last time latest was pushed was yesterday morning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: Long Running Lambda Fails StepFunction State Machine Execution
4 participants