Skip to content

Sns:v2 platform application crud #13312

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 6 commits into from
Oct 29, 2025
Merged

Sns:v2 platform application crud #13312

merged 6 commits into from
Oct 29, 2025

Conversation

@baermat
Copy link
Member

@baermat baermat commented Oct 28, 2025 •
edited
Loading

Motivation

This PR adds the crud methods for both Platform Applications as well as platform applications attributes

Changes

  • adds create_platform_application
  • adds list_platform_applications
  • adds delete_platform_application
  • adds get_platform_application_attributes
  • adds set_platform_application_attributes
  • adds stub for list_endpoints_for_application_attributes (will be implemented with endpoints)
  • add new PlatformApplicationCrud test class with tests

Tests

Related

closes PNX-85, closes PNX-81

@baermat baermat added semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases docs: skip Pull request does not require documentation changes notes: skip Pull request does not have to be mentioned in the release notes labels Oct 28, 2025
@github-actions
Copy link

github-actions bot commented Oct 28, 2025 •
edited
Loading

Test Results - Preflight, Unit

22 376 tests  ±0   20 626 ✅ ±0   16m 2s ⏱️ -31s
     1 suites ±0    1 750 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 535a1d1. ± Comparison against base commit 99973f7.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Oct 28, 2025 •
edited
Loading

Test Results - Alternative Providers

176 tests    56 ✅  37s ⏱️
  1 suites  120 💤
  1 files      0 ❌

Results for commit 535a1d1.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Oct 28, 2025 •
edited
Loading

Test Results (amd64) - Acceptance

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

Results for commit 535a1d1. ± Comparison against base commit 99973f7.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Oct 28, 2025 •
edited
Loading

Test Results (amd64) - Integration, Bootstrap

    5 files      5 suites   2h 39m 45s ⏱️
5 268 tests 4 739 ✅ 529 💤 0 ❌
5 274 runs  4 739 ✅ 535 💤 0 ❌

Results for commit 535a1d1.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Oct 28, 2025 •
edited
Loading

LocalStack Community integration with Pro

    2 files      2 suites   2h 2m 0s ⏱️
4 894 tests 4 525 ✅ 369 💤 0 ❌
4 896 runs  4 525 ✅ 371 💤 0 ❌

Results for commit 535a1d1.

♻️ This comment has been updated with latest results.

@baermat baermat marked this pull request as ready for review October 28, 2025 12:11
@baermat baermat requested a review from bentsku as a code owner October 28, 2025 12:11
bentsku
bentsku reviewed Oct 28, 2025
View reviewed changes
Copy link
Contributor

@bentsku bentsku left a comment

Choose a reason for hiding this comment

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

Wow, this is nice! Congrats for being able to validate those tests, this is awesome!

I only have a few comments, mostly related to the manual setup required, if we could centralize how we are using the credentials I think it'd be easier to re-validate all tests (instead of manually modifying those credentials by hand every time), and some nits.

This is a really nice PR! 🥳

localstack-core/localstack/services/sns/v2/utils.py
return _parse_and_validate_arn(platform_application_arn, "PlatformApplication")


def _parse_and_validate_arn(arn: str | None, resource_type: str) -> ArnData:
Copy link
Contributor

Choose a reason for hiding this comment

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

praise: nice adapting this function to be reused!

tests/aws/services/sns/test_sns.py
assert messages[0]["Source"] == sender_address


class TestSNSPlatformApplicationCrud:
Copy link
Contributor

Choose a reason for hiding this comment

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

congrats on validating those tests, this is really awesome!

@baermat baermat requested a review from bentsku October 29, 2025 09:33
bentsku
bentsku approved these changes Oct 29, 2025
View reviewed changes
Copy link
Contributor

@bentsku bentsku left a comment

Choose a reason for hiding this comment

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

LGTM, this is really great, thanks for addressing the comments super fast, this is awesome! Really nice PR 🥳

tests/aws/services/sns/test_sns.py
self, aws_client, snapshot, sns_create_platform_application, platform_credentials
):
platform = "ADM"
# if tested against AWS, the fixture needs to contain real credentials
Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for adding the comment, this is very clear 👍

@baermat baermat merged commit d87d075 into main Oct 29, 2025
43 checks passed
@baermat baermat deleted the sns/v2-platform-application-crud branch October 29, 2025 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bentsku bentsku bentsku approved these changes

Assignees

No one assigned

Labels

docs: skip Pull request does not require documentation changes notes: skip Pull request does not have to be mentioned in the release notes semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@baermat @bentsku