Skip to content

Improve SFN store #13550

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.

Sign up for GitHub

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

Jump to bottom
Draft
giograno wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Improve SFN store #13550

giograno wants to merge 3 commits into from

Conversation

@giograno
Copy link
Member

@giograno giograno commented Dec 19, 2025 •
edited
Loading

Motivation

As I tried to get an Avro schema from SFNStore, I came across a few issues:

  • a few fields with typing.Any as type hints;
  • a model class having a boto client as an attribute;
  • a model class using a deque;
  • the Execution class having an ExecutionWorker as an attribute. The general rule is to try to separate the runtime state from the state that needs to be persisted. Furthermore, ExecutionWorker is generally harder to serialize since it holds threading primitives.

Changes

  • Fix the typing.Any annotations;
  • Move the boto client as property of the class;
  • Using a list instead of deque. We are going from O(1) to O(n) for the get_task operation;
  • I removed the exec_comm attribute from the Executor. An executor now needs to register a worker when it starts it, to be able to retrieve it later with the execution ARN.

Tests

Related

@giograno giograno self-assigned this Dec 19, 2025
@giograno giograno added area: persistence Retain state between LocalStack runs aws:stepfunctions AWS Step Functions docs: skip Pull request does not require documentation changes notes: skip Pull request does not have to be mentioned in the release notes labels Dec 19, 2025
@github-actions
Copy link

github-actions bot commented Dec 19, 2025 •
edited
Loading

Test Results - Preflight, Unit

22 982 tests  ±0   21 140 ✅ ±0   6m 10s ⏱️ +9s
     1 suites ±0    1 842 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit dfac85d. ± Comparison against base commit 056da83.

♻️ This comment has been updated with latest results.

@giograno giograno added the semver: patch Non-breaking changes which can be included in patch releases label Dec 19, 2025
@github-actions
Copy link

github-actions bot commented Dec 19, 2025 •
edited
Loading

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   3m 4s ⏱️ +15s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit dfac85d. ± Comparison against base commit 056da83.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Dec 19, 2025 •
edited
Loading

LocalStack Community integration with Pro

    2 files  ±    0      2 suites  ±0   22m 15s ⏱️ - 1h 34m 57s
1 721 tests  - 3 432  1 632 ✅  - 3 124  89 💤  - 308  0 ❌ ±0 
1 723 runs   - 3 432  1 632 ✅  - 3 124  91 💤  - 308  0 ❌ ±0 

Results for commit dfac85d. ± Comparison against base commit 056da83.

This pull request removes 3432 tests. 
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_lambda_dynamodb
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_opensearch_crud
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_search_books
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_setup
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_destination_sns
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_infra
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_prefill_dynamodb_table
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input0-SUCCEEDED]
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input1-SUCCEEDED]
…

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Dec 19, 2025 •
edited
Loading

Test Results (amd64) - Integration, Bootstrap

    5 files      5 suites   39m 10s ⏱️
1 745 tests 1 657 ✅ 88 💤 0 ❌
1 751 runs  1 657 ✅ 94 💤 0 ❌

Results for commit dfac85d.

♻️ This comment has been updated with latest results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dominikschubert dominikschubert Awaiting requested review from dominikschubert dominikschubert will be requested when the pull request is marked ready for review dominikschubert is a code owner

@joe4dev joe4dev Awaiting requested review from joe4dev joe4dev will be requested when the pull request is marked ready for review joe4dev is a code owner

@gregfurman gregfurman Awaiting requested review from gregfurman gregfurman will be requested when the pull request is marked ready for review gregfurman is a code owner

@tiurin tiurin Awaiting requested review from tiurin tiurin will be requested when the pull request is marked ready for review tiurin is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@giograno giograno

Labels

area: persistence Retain state between LocalStack runs aws:stepfunctions AWS Step Functions docs: skip Pull request does not require documentation changes notes: skip Pull request does not have to be mentioned in the release notes semver: patch Non-breaking changes which can be included in patch releases

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@giograno