Skip to content

StepFunctions: Fix Boto Request Encoding #12035

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
Merged
merged 1 commit into from
Dec 13, 2024
Merged

StepFunctions: Fix Boto Request Encoding #12035

merged 1 commit into from
Dec 13, 2024

Conversation

MEPalma
Copy link
Contributor

@MEPalma MEPalma commented Dec 13, 2024

Motivation

The SFN v2 interpreter incorporates custom logic to convert SFN request parameter objects to ensure compatibility with boto requests, and vice versa. However, this logic currently misencodes JSON strings, occasionally resulting in corrupted JSON objects being sent to the target service #12030. These changes rectify this issue.

Changes

  • Fix json object encoding separators and ensure strings are also converted to json string literals
  • Updated s3 service integration tests to sample the resulting value for multiple json types

@MEPalma MEPalma added the semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases label Dec 13, 2024
@MEPalma MEPalma added this to the 4.1 milestone Dec 13, 2024
@MEPalma MEPalma self-assigned this Dec 13, 2024
@github-actions GitHub Actions
Copy link

LocalStack Community integration with Pro

    2 files  ±    0      2 suites  ±0   36m 1s ⏱️ - 1h 17m 46s
1 333 tests  - 2 561  1 269 ✅  - 2 315  64 💤  - 246  0 ❌ ±0 
1 335 runs   - 2 561  1 269 ✅  - 2 315  66 💤  - 246  0 ❌ ±0 

Results for commit 788e321. ± Comparison against base commit cb66b15.

This pull request removes 2561 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]
…

gregfurman
gregfurman approved these changes Dec 13, 2024
View reviewed changes
Copy link
Contributor

@gregfurman gregfurman left a comment

Choose a reason for hiding this comment

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

LGTM 🫡

@MEPalma MEPalma merged commit 235a06a into master Dec 13, 2024
36 checks passed
@MEPalma MEPalma deleted the MEP-sfn-s3_put_json_enc branch December 13, 2024 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gregfurman gregfurman gregfurman approved these changes

@joe4dev joe4dev Awaiting requested review from joe4dev

@dominikschubert dominikschubert Awaiting requested review from dominikschubert

Assignees

@MEPalma MEPalma

Labels
semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases
Projects
None yet
Milestone
4.1
Development

Successfully merging this pull request may close these issues.
2 participants
@MEPalma @gregfurman