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.

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

fix SNS publish across region exception #9679

Merged
merged 4 commits into from Nov 23, 2023
Merged

fix SNS publish across region exception #9679

merged 4 commits into from Nov 23, 2023

Conversation

bentsku
Copy link
Contributor

@bentsku bentsku commented Nov 17, 2023 •
edited

Motivation

Some users had issues with region in another issue, but their trace logs showed another issue badly handled by SNS when the topic didn't exist and wasn't properly checked.

Changes

Added better check and a AWS validated test when trying to publish from another region.

@bentsku bentsku added aws:sns Amazon Simple Notification Service semver: patch Non-breaking changes which can be included in patch releases labels Nov 17, 2023
@bentsku bentsku self-assigned this Nov 17, 2023
@coveralls
Copy link

coveralls commented Nov 17, 2023 •
edited

Coverage Status

coverage: 84.015% (+0.01%) from 84.003%
when pulling de631d5 on fix-sns-topic-attrs
into 32a7946 on master.

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 17, 2023 •
edited

LocalStack Community integration with Pro

       2 files         2 suites   1h 4m 28s ⏱️
2 324 tests 2 025 ✔️ 299 💤 0 ❌
2 325 runs  2 025 ✔️ 300 💤 0 ❌

Results for commit de631d5.

♻️ This comment has been updated with latest results.

@bentsku bentsku marked this pull request as ready for review November 17, 2023 21:05
@bentsku bentsku requested a review from thrau as a code owner November 17, 2023 21:05
@bentsku bentsku requested review from baermat and removed request for thrau November 17, 2023 21:05
@bentsku bentsku marked this pull request as draft November 17, 2023 21:07
@bentsku bentsku marked this pull request as ready for review November 17, 2023 21:23
viren-nadkarni
viren-nadkarni approved these changes Nov 23, 2023
View reviewed changes
Copy link
Member

@viren-nadkarni viren-nadkarni left a comment

Choose a reason for hiding this comment

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

LGTM ✔️

@bentsku bentsku merged commit d94a71a into master Nov 23, 2023
26 checks passed
@bentsku bentsku deleted the fix-sns-topic-attrs branch November 23, 2023 17:22
@bentsku bentsku mentioned this pull request Nov 24, 2023
Closed
1 task
@viren-nadkarni viren-nadkarni mentioned this pull request Dec 15, 2023
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@viren-nadkarni viren-nadkarni viren-nadkarni approved these changes

@baermat baermat baermat approved these changes

Assignees

@bentsku bentsku

Labels
aws:sns Amazon Simple Notification Service 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.

None yet
4 participants
@bentsku @coveralls @viren-nadkarni @baermat