feat(sns): add notExistsCondition method#34712
feat(sns): add notExistsCondition method#34712mergify[bot] merged 15 commits intoaws:mainfrom jaw111:34707-sns-subscription-filter-exists
Conversation
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
| denylist: ['white', 'orange'], | ||
| })), | ||
| }), | ||
| price: sns.Filter.filter(sns.SubscriptionFilter.numericFilter({ |
There was a problem hiding this comment.
Clarification Request:
Does it make sense to specify multiple numeric filters that contradict each other? I assume the message has to match all filter rules to be sent to the SQS queue (logic is AND not OR), but the allowed values 100 and 200 will not satisfy the other numeric filters.
There was a problem hiding this comment.
After reading the documentation, it appears the OR logic would apply. This means the filter would match any numeric value due to the greaterThan: 500 and lessThan: 1000 constraints.
So, it depends on what the intended purpose of such an integration test is. Should it just check the CloudFormation stack is built and deployed, or also test the behavior of the filter does what is expected and messages get filtered accordingly to subscribers?
packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-sqs.ts
Show resolved
Hide resolved
|
@pahud @badmintoncryer sorry to pester on this, but do you have any timeline when someone will be able to review this PR? |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Comments on closed issues and PRs are hard for our team to see. |
Issue # (if applicable)
Closes #34707
Reason for this change
Set subscription filter with exists
falsefor a property.Description of changes
Add new
notExistsCondition()method.Describe any new or updated permissions being added
None.
Description of how you validated changes
Extended existing integration test.
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license