Skip to content

SNS: support passing MessageGroupId to non-FIFO topic #13019

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
Aug 19, 2025

Conversation

bentsku
Copy link
Contributor

@bentsku bentsku commented Aug 18, 2025

Motivation

Related to #13008 -> https://aws.amazon.com/about-aws/whats-new/2025/07/amazon-sns-standard-topics-sqs-fair-queues/

Since adding support for SQS Fair Queues, we can now pass a MessageGroupId to non-FIFO topic.

This PR implements that behavior in SNS, removing the previously raised exception, and properly passing the attribute.

This also fixes the behavior in SQS itself following #12930, because we were not adding the MessageGroupId to the Attributes of the received Message, as this only done for FIFO queue. I've updated the SQS test to verify this

Changes

  • remove raising an exception for non-FIFO topics
  • remove the check for FIFO topic before passing the attribute as kwarg to SQS.SendMessage
  • fix the behavior in SQS to return the MessageGroupId as Attributes of the message
  • update SQS test

@bentsku bentsku added this to the 4.8 milestone Aug 18, 2025
@bentsku bentsku self-assigned this Aug 18, 2025
@bentsku bentsku added aws:sqs Amazon Simple Queue Service aws:sns Amazon Simple Notification Service semver: patch Non-breaking changes which can be included in patch releases labels Aug 18, 2025
@bentsku bentsku force-pushed the sns/implement-msg-group-id branch from 007ecdb to 227c351 Compare August 18, 2025 15:37
Copy link

github-actions bot commented Aug 18, 2025

Test Results - Preflight, Unit

22 139 tests   20 404 ✅  6m 27s ⏱️
     1 suites   1 735 💤
     1 files         0 ❌

Results for commit 8a98ce0.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Aug 18, 2025

Test Results (amd64) - Acceptance

7 tests   5 ✅  3m 10s ⏱️
1 suites  2 💤
1 files    0 ❌

Results for commit 8a98ce0.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Aug 18, 2025

Test Results (amd64) - Integration, Bootstrap

    5 files      5 suites   1h 31m 18s ⏱️
3 231 tests 3 099 ✅ 132 💤 0 ❌
3 237 runs  3 099 ✅ 138 💤 0 ❌

Results for commit 8a98ce0.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Aug 18, 2025

LocalStack Community integration with Pro

    2 files      2 suites   1h 3m 6s ⏱️
3 202 tests 3 067 ✅ 135 💤 0 ❌
3 210 runs  3 073 ✅ 137 💤 0 ❌

Results for commit 8a98ce0.

♻️ This comment has been updated with latest results.

@bentsku bentsku marked this pull request as ready for review August 18, 2025 17:41
Copy link
Member

@thrau thrau left a comment

Choose a reason for hiding this comment

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

that's a cool new feature! LGTM

Comment on lines +774 to +778
standard_message = SqsMessage(
time.time(),
message,
message_group_id=message_group_id,
)
Copy link
Member

Choose a reason for hiding this comment

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

thanks for catching and fixing this!

@bentsku bentsku merged commit 57a8f25 into main Aug 19, 2025
41 checks passed
@bentsku bentsku deleted the sns/implement-msg-group-id branch August 19, 2025 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws:sns Amazon Simple Notification Service aws:sqs Amazon Simple Queue Service 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.

2 participants