Skip to content

[SFN] SNS Optimised Integration: Automatic Stringification of Message Arguments #10165

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

Merged
merged 3 commits into from
Feb 5, 2024

Conversation

MEPalma
Copy link
Contributor

@MEPalma MEPalma commented Feb 1, 2024

Motivation

Currently, the StepFunctions v2 interpreter lacks support for automatic stringification of message arguments in the SNS Optimised Integration. This limitation can lead to unintended behaviour in publish calls. This PR addresses such lacking support and adds relevant validated snapshot tests for SNS integrations, including a callback scenario.

Changes

  • Added automatic stringification of message arguments in the optimised SNS service integrations
  • Added relevant snapshot tests

@MEPalma MEPalma added the semver: patch Non-breaking changes which can be included in patch releases label Feb 1, 2024
@MEPalma MEPalma self-assigned this Feb 1, 2024
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.

minor nit, otherwise LGTM

Really appreciate the ultra fast turnaround time on this 💨

Comment on lines 90 to 93
if "Message" in normalised_parameters:
message = normalised_parameters["Message"]
if not isinstance(message, str):
normalised_parameters["Message"] = to_json_str(message)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if "Message" in normalised_parameters:
message = normalised_parameters["Message"]
if not isinstance(message, str):
normalised_parameters["Message"] = to_json_str(message)
if message := normalized_parameters.get("Message"):
if not isinstance(message, str):
normalized_parameters["Message"] = to_json_str(message)

bit of a nit (walrus operator)

also we generally try to use American English for consistency

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, Dominik! The walrus operator really improves readability in this case. I changed it slightly to ensure it wouldn't add missing 'Message' parameters (in case this is not given by the program).

About the British spelling, I realise it's a problem for this provider, I'll open a PR to standardise all of it!

Comment on lines +1493 to +1496
"Message": {
"arg2": "World",
"arg1": "Hello",
"TaskToken": "<task_token:1>"
Copy link
Member

Choose a reason for hiding this comment

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

🥳 nice

@coveralls
Copy link

Coverage Status

coverage: 83.845% (+0.009%) from 83.836%
when pulling 0d250af on MEP-sfn-sns_payload_waitForTaskToken
into f3811b7 on master.

@MEPalma MEPalma merged commit fee871a into master Feb 5, 2024
@MEPalma MEPalma deleted the MEP-sfn-sns_payload_waitForTaskToken branch February 5, 2024 08:21
maxhoheiser pushed a commit that referenced this pull request Feb 5, 2024
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.

3 participants